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.

Sign up for GitHub

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

Jump to bottom

Export SLEAP .h5 analysis files #88

Merged
merged 15 commits into from
Dec 5, 2023
Merged

Export SLEAP .h5 analysis files #88

merged 15 commits into from
Dec 5, 2023

Conversation

lochhh
Copy link
Collaborator

@lochhh lochhh commented Nov 22, 2023 •
edited
Loading

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
This PR enables movement datasets to be exported as SLEAP-style .h5 files.

What does this PR do?

  1. Adds:
    • to_sleap_analysis_file which converts an xarray pose tracks dataset (cleaned using _remove_unoccupied_tracks) to a SLEAP-style .h5 analysis file, and the relevant documentation to the API docs
    • documentation to "saving data" in "Getting started"
    • file path and dataset validation functions in save_poses.py
    • relevant tests for the new functions, and rewrites reused test parameters as fixtures
  2. Replaces the broken pooch URL with their GitHub repository URL to pass linkcheck when building docs.
  3. Refactors file and file path validation in save_poses.py as reusable functions.
  4. Reorganises test fixtures by moving related (and potentially reusable) fixtures into conftest.py .

References

This PR resolves #46.

How has this PR been tested?

  • Added new tests
  • Reran all tests

Does this PR require an update to the documentation?

  • Saving data example in "getting started" has been updated
  • Added to_sleap_analysis_file to API docs

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@codecov Codecov
Copy link

codecov bot commented Nov 22, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (88b23de) 98.37% compared to head (6bef6c1) 98.51%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
+ Coverage   98.37%   98.51%   +0.13%     
==========================================
  Files           8        8              
  Lines         369      403      +34     
==========================================
+ Hits          363      397      +34     
  Misses          6        6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lochhh lochhh marked this pull request as ready for review November 23, 2023 15:14
@lochhh lochhh requested a review from niksirbi November 23, 2023 18:58
niksirbi
niksirbi approved these changes Nov 24, 2023
View reviewed changes
Copy link
Member

@niksirbi niksirbi left a comment

Choose a reason for hiding this comment

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

Great work as usual @lochhh. I've approved this one, but I will withhold merging until I've also reviewd PR #90, since these two are intimately related.

docs/source/getting_started.md Outdated Show resolved Hide resolved
movement/io/save_poses.py Outdated Show resolved Hide resolved
movement/io/save_poses.py Show resolved Hide resolved
movement/io/save_poses.py Outdated Show resolved Hide resolved
movement/io/save_poses.py Show resolved Hide resolved
movement/io/save_poses.py Show resolved Hide resolved
tests/test_unit/test_save_poses.py Outdated Show resolved Hide resolved
@lochhh lochhh mentioned this pull request Dec 4, 2023
Open
@lochhh lochhh requested a review from niksirbi December 4, 2023 17:32
niksirbi
niksirbi approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@niksirbi niksirbi left a comment

Choose a reason for hiding this comment

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

LGTM! I only left one comment, but I'm not even sure whether it's worth addressing. Up to you to merge when ready.

tests/test_integration/test_io.py
"SLEAP_three-mice_Aeon_mixed-labels.predictions.slp",
],
)
def test_to_sleap_analysis_file_source_file(self, file, new_h5_file):
Copy link
Member

Choose a reason for hiding this comment

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

This test seems a bit over-parametrised to me.
Is it really necessary to test all of these cases? It doesn't hurt to be cautious I guess, but I'm a bit worried over the long-term impact on our test time (if we keep being so thorough).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, good catch! I was just checking if it works in all cases. I've removed all but one test case per file type supported for each of DLC and SLEAP.

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Dec 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@lochhh lochhh merged commit d0b3be6 into main Dec 5, 2023
24 checks passed
@lochhh lochhh deleted the export-sleap-h5 branch December 5, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niksirbi niksirbi niksirbi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Export pose tracks to SLEAP analysis file
2 participants
@lochhh @niksirbi