Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ndx-pose>=0.2 for DeepLabCutInterface and LightningPoseDataInterface #1128

Merged
merged 27 commits into from
Jan 23, 2025

Conversation

h-mayorquin
Copy link
Collaborator

This uses the latest release of ndx-pose and modifies the DeepLabCutInterface class to use the latest API (e.g. Skeletons instead of nodes).

This is related to both:
#1114
and
#1127

@h-mayorquin h-mayorquin self-assigned this Nov 6, 2024
@pauladkisson
Copy link
Member

@h-mayorquin, are there any blockers on this? Having this done will help me ensure that the dev tests are fixed properly.

@h-mayorquin
Copy link
Collaborator Author

h-mayorquin commented Dec 13, 2024

We are waiting for sleap.io to review the PR:

talmolab/sleap-io#143

If having this would make your life easier I propose

  • You can update LightningPose and upgrade it.
  • Then I can modify the CI so we test sleap interface as a stand-alone test while the PR above gets merged.

Or if you have another proposal I am all ears.

@pauladkisson
Copy link
Member

We are waiting for sleap.io to review the PR:

I'm a tad confused bc this PR updates DLC and LightningPose but not sleap.

But in general I am in favor of merging what we can, updating the CLI to isolate SLEAP and then we can re-integrate it once the PR is reviewed.

@h-mayorquin
Copy link
Collaborator Author

h-mayorquin commented Dec 16, 2024

I'm a tad confused bc this PR updates DLC and LightningPose but not sleep.

The PR does not update the code of LightingPos only the one of DLC.

  • The code of LightningPose needs to be updated
  • We need the upstream changes on sleep but for that, we can adjust on the CI temporary.

@pauladkisson
Copy link
Member

The PR does not update the code of LightingPos only the one of DLC.

Ah, I see. You're just resolving dependency conflicts with LightningPose.

Well, even better. We can update DLC in this PR and then LightningPose in the next, adding the appropriate skipifs. Then when all the ndx-pose interfaces are updated we can re-integrate the CI.

@h-mayorquin
Copy link
Collaborator Author

Yeah, that makes sense to me.

Can you review #1158? I want to merge that one then do a release. I want to do a release before merging this PR and the one in #1153 as they are both large changes on behavior.

@h-mayorquin h-mayorquin changed the title Use latest version of ndx pose for DeepLabCutInterface Use latest version of ndx pose for DeepLabCutInterface and LightningPoseDataInterface Jan 21, 2025
@talmo
Copy link

talmo commented Jan 21, 2025

talmolab/sleap-io#143 merged!

Will ping when we cut a new release.

@h-mayorquin
Copy link
Collaborator Author

Thanks a lot @talmo !

@h-mayorquin h-mayorquin marked this pull request as ready for review January 21, 2025 21:01
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new file with only the pose estimation interfaces.

@h-mayorquin h-mayorquin changed the title Use latest version of ndx pose for DeepLabCutInterface and LightningPoseDataInterface Use ndx-pose>=0.2 for DeepLabCutInterface and LightningPoseDataInterface Jan 22, 2025
Copy link
Member

@pauladkisson pauladkisson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small suggestions, but otherwise looks good!

CHANGELOG.md Outdated Show resolved Hide resolved
@h-mayorquin h-mayorquin enabled auto-merge (squash) January 23, 2025 19:45
@h-mayorquin h-mayorquin merged commit b787708 into main Jan 23, 2025
40 checks passed
@h-mayorquin h-mayorquin deleted the use_latest_version_of_ndx_pose branch January 23, 2025 22:56
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 73.77049% with 16 lines in your changes missing coverage. Please review.

Project coverage is 89.64%. Comparing base (96dfdff) to head (ee83879).
Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
...atainterfaces/behavior/sleap/sleapdatainterface.py 0.00% 6 Missing ⚠️
...v/datainterfaces/behavior/deeplabcut/_dlc_utils.py 80.76% 5 Missing ⚠️
...havior/lightningpose/lightningposedatainterface.py 80.00% 4 Missing ⚠️
...ces/behavior/deeplabcut/deeplabcutdatainterface.py 88.88% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1128      +/-   ##
==========================================
- Coverage   90.69%   89.64%   -1.05%     
==========================================
  Files         129      129              
  Lines        8189     8345     +156     
==========================================
+ Hits         7427     7481      +54     
- Misses        762      864     +102     
Flag Coverage Δ
unittests 89.64% <73.77%> (-1.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/neuroconv/basedatainterface.py 87.05% <ø> (-8.30%) ⬇️
...ces/behavior/deeplabcut/deeplabcutdatainterface.py 92.45% <88.88%> (-1.03%) ⬇️
...havior/lightningpose/lightningposedatainterface.py 92.80% <80.00%> (-2.58%) ⬇️
...v/datainterfaces/behavior/deeplabcut/_dlc_utils.py 61.01% <80.76%> (+2.12%) ⬆️
...atainterfaces/behavior/sleap/sleapdatainterface.py 40.38% <0.00%> (-55.27%) ⬇️

... and 19 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants