Splitting_Individuals_and_DLC_Format #529
Conversation
|
|
Hii @niksirbi Thankyou |
Thanks for starting to work on this @Mahi7828! In the meantime, see if you can modify the tests for this function. You will find these in |
There was a problem hiding this comment.
Thanks for giving this a shot @Mahi7828.
I think the current state of this PR is still far from being merged, so I'll convert this to a Draft PR.
Apart from the issues I've highlighted in my review, there are also several test failures which need to be addressed. Besides that, the PR has to be synced with the current main branch (via a merge or a rebase), existing tests should be modified (and maybe new ones added) to check whether the modified behaviour works as intended, and documentation needs to be updated in the Input/Output guide.
Once you’ve addressed these, you can mark the PR as ready for review again. Let me know whether you’d like to continue working on this or if you’d prefer me to pick it up—either is fine!
Thanks again for your efforts,
Niko
| """ | ||
| # Concatenate the pose tracks and confidence scores into one array | ||
| """Convert a ``movement`` dataset to a DLC-style DataFrame.""" |
There was a problem hiding this comment.
You seem to have (inadvertedly?) deleted a big part of the docstring. The docstring should be restored to its previouse state.
| print("Position shape:", position_shape) | ||
| print("Confidence shape:", confidence_shape) | ||
|
|
||
| # Concatenate the pose tracks and confi scores into one array |
| print("Position shape:", position_shape) | ||
| print("Confidence shape:", confidence_shape) |
There was a problem hiding this comment.
Print statements can be useful for debugging but we don't want them in the final code. Please remove these and all other print statements you've introduced.
| # Check the shape of the data | ||
| expected_columns = columns.shape[0] | ||
| actual_shape = tracks_with_scores.reshape(ds.sizes["time"], -1).shape[1] | ||
|
|
||
| if actual_shape != expected_columns: | ||
| raise ValueError(f"""Shape of passed values is {actual_shape}, | ||
| but indices imply {expected_columns}.""") |
There was a problem hiding this comment.
Not sure why this additional check was introduced here. Also, we prefer to log our error messages, as decribed in the contributing guide here: https://movement.neuroinformatics.dev/community/contributing.html#logging
| split_individuals : bool, optional | ||
| If True, return a dictionary of DataFrames per individual, with | ||
| individual names as keys. If False (default), return a single | ||
| DataFrame for all individuals (see Notes). | ||
| DataFrame for all individuals. | ||
| dlc_df_format : {"single-animal", "multi-animal"}, optional | ||
| Specifies the DLC dataframe format. "single-animal" produces the | ||
| older format (<DLC 2.0) without the "individuals" column level, | ||
| while "multi-animal" includes it (DLC >=2.0). | ||
| Defaults to "multi-animal". |
There was a problem hiding this comment.
It's nice that we can now specify the DLC dataframe format independently of splitting, but there is one combination which doesn't make sense: datasets with a single individual are incompatible with split_individuals=True, i.e. "splitting" doesn't make sense if there's nothing to split. If a user passes split_individuals=True for a single-animal dataset, we should treat split_individuals as False and raise a warning to the user, with warnings.warn.
There was a problem hiding this comment.
This case should be caught in the code and also mentioned in the docstring.
| See Also | ||
| -------- | ||
| to_dlc_file : Save dataset directly to a DeepLabCut-style .h5 or .csv file. | ||
There was a problem hiding this comment.
You seem to have deleted the docstrign sections for "Notes" and "See Also". The should be brought back and the Notes adjusted according to the new implementation.
| split_individuals : bool or "auto", optional | ||
| Whether to save individuals to separate files or to the same file | ||
| (see Notes). Defaults to "auto". | ||
| Whether to save individuals to separate files or to the same file. | ||
| Defaults to "auto" (determined based on dataset individuals). | ||
| dlc_df_format : {"single-animal", "multi-animal"}, optional | ||
| Specifies the DLC dataframe format. "single-animal" produces the | ||
| older format (<DLC 2.0) without the "individuals" column level, | ||
| while "multi-animal" includes it (DLC >=2.0). | ||
| Defaults to "multi-animal". | ||
| Notes | ||
| ----- | ||
| If ``split_individuals`` is True, each individual will be saved to a | ||
| separate file, formatted as in a single-animal DeepLabCut project | ||
| (without the "individuals" column level). The individual's name will be | ||
| appended to the file path, just before the file extension, e.g. | ||
| "/path/to/filename_individual1.h5". If False, all individuals will be | ||
| saved to the same file, formatted as in a multi-animal DeepLabCut project | ||
| (with the "individuals" column level). The file path will not be modified. | ||
| If "auto", the argument's value is determined based on the number of | ||
| individuals in the dataset: True if there is only one, False otherwise. | ||
| See Also | ||
| -------- | ||
| to_dlc_style_df : Convert dataset to DeepLabCut-style DataFrame(s). | ||
| Examples | ||
| -------- | ||
| >>> from movement.io import save_poses, load_poses | ||
| >>> ds = load_poses.from_sleap_file("/path/to/file_sleap.analysis.h5") | ||
| >>> save_poses.to_dlc_file(ds, "/path/to/file_dlc.h5") |
There was a problem hiding this comment.
Also here you've deleted the See Also and Examples sections, they need to be brought back.
| if dlc_df_format not in ["single-animal", "multi-animal"]: | ||
| raise log_error( | ||
| ValueError, | ||
| f"""Invalid value for 'dlc_df_format': {dlc_df_format}. | ||
| Expected 'single-animal' or 'multi-animal'.""", | ||
| ) |
There was a problem hiding this comment.
Use the updated way for logging errors, see https://movement.neuroinformatics.dev/community/contributing.html#logging
|
Hi @niksirbi, |
2 participants
Before submitting a pull request (PR), please read the contributing guide.
Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)
Description
What is this PR
Why is this PR needed?
To allow the users to select the DLC version according to their choice when saving poses for multi or single individuals.
What does this PR do?
References
Please reference any existing issues/PRs that relate to this PR.
How has this PR been tested?
I tried working on the save_poses.py file and had changed the _ds_to_dlc_style_df() and to_dlc_style_df() function, but the pytest are giving some errors about the shapes, which I am currently not able to figure out.
Is this a breaking change?
No
Does this PR require an update to the documentation?
Yes
Checklist: