"deeplabcut.filterpredictions" and "deeplabcut.analyze_skeleton" return mappings #1908
Conversation
there were two try/excepts, one within the other. this commit simplifies the code by pulling the inner try/except out. the code now tries to look for filtered data. if filtered data is found for the video, we continue to the next item in the loop. if not found, we catch and ignore the error. we then try to load the data and filter it modified: deeplabcut/post_processing/filtering.py
the second try block in filterpredictions looked for the analyzed file and performed operations to filter it - and if the file is not found, continue to the next item in the for loop. this commit reduces the try block to only load the analyzed file and immediately catch the error and continue if the file doesnt exist. the rest of the filtering on the analyzed data is performed outside the try/except block. modified: deeplabcut/post_processing/filtering.py
the filterpredictions now returns a mapping instead of returning None. modified: deeplabcut/post_processing/filtering.py
rahulporuri
changed the title
instead of finding and analyzing the skeleton in the try block, this commit reduces the scope of the try block to only finding the file and moves the skeleton calculations to outside the try/except. modified: deeplabcut/post_processing/analyze_skeleton.py
modified: deeplabcut/post_processing/filtering.py
the analyze_skeleton function now returns a mapping instead of None modified: deeplabcut/post_processing/analyze_skeleton.py
|
Thanks @rahulporuri! Looks great to me! |
|
Thanks for the quick review @jeylau .
I can't go into the full details but like I mentioned in #1895 , we're using DeepLabCut within a larger application. This specific PR allows us to directly get data back from the API instead of reading it back from files. I wanted to go further and add additional API kwargs to prevent the files from being saved but I didn't want to propose more changes than were necessary. |
|
@jeylau do you know if there's any plan to normalize naming in the codebase e.g. renaming functions to use underscores instead of CamelCase naming convention in |
|
Renaming functions to snake case is something we gradually do when refactoring/cleaning code. We're definitely more than happy to get a bit of help here too! 😄 |
|
Good addition, but I think we should make flag: |
I'd actually flip it the other way around - add a |
|
I think it's best to set the flags so that the functionality is stable with respect to prior versions. |
|
hi all, what's the decision on this PR? |
|
shall we add the flags cc @AlexEMG @rahulporuri @n-poulsen ? or close? |
|
Bump to dev team! ^ |
|
I think adding the flags is a great idea |
|
@n-poulsen can you add the flag so we can merge/close this PR? |
|
@AlexEMG @MMathisLab flag added, good to merge now! Thanks @rahulporuri, sorry for getting back to your PR only now! |
I apologize for not making the time to come back to my PR. Thank you very much for continuing to work on it! |
5 participants
deeplabcut.filterpredictionsat the moment returnsNone. Internally, there are twotry/exceptblocks. The first try/except block looks for existing filtered data. The second try/except block looks for analyzed data to filter.This PR does the following -
Noneis returned for the corresponding video filename.Similarly,
deeplabcut.analyze_skeletonat the moment returnsNone.Similar to change in
deeplabcut.filterpredictionsearlier, the scope of thetry/exceptis also reduced - now the skeleton calculations are performed outside the try/except block. Anddeeplabcut.analyze_skeletonreturns a mapping from the video filename to the corresponding skeleton data. If analyzed data isn't found for a given video filename,Nonewill be used and if a pre-existing skeleton data file is found, a dataframe will be loaded from the file.This addresses one of the points mentioned in #1895 -
deeplabcutfunctions returning information instead of returningNone.Note to reviewer : It might be easier to review this PR one-commit-at-a-time.