-
Notifications
You must be signed in to change notification settings - Fork 96
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
Fix IndexError for hdf5 file import for single instance analysis files #1695
Conversation
WalkthroughA new test function was added to Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sleap/io/format/sleap_analysis.py (1 hunks)
Additional comments: 1
sleap/io/format/sleap_analysis.py (1)
- 81-81: The added condition to check if "track_names" exists and is not empty before proceeding is a good approach to handle the scenario where track names are missing in single instance analysis files. This fix should prevent the
IndexError
that was occurring due to an emptytrack_names
list. However, it's important to ensure that the default track names generated in theelse
block (f"track_{i}"
) are suitable and consistent with the rest of the application's naming conventions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1695 +/- ##
========================================
Coverage 73.38% 73.38%
========================================
Files 134 134
Lines 24064 24064
========================================
+ Hits 17659 17660 +1
+ Misses 6405 6404 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
tests/data/videos/single_fly.mp4
is excluded by:!**/*.mp4
Files selected for processing (2)
- tests/fixtures/videos.py (2 hunks)
- tests/io/test_formats.py (1 hunks)
Additional comments: 7
tests/fixtures/videos.py (5)
- 4-4: Import of
FileHandle
added.This import is necessary for handling file operations with the newly added fixture
single_fly_hdf5
.
- 11-11: Added constant
TEST_SINGLE_FLY_H5
.This constant provides a clear and reusable reference to the path of the single fly HDF5 file used in tests.
- 14-16: Added fixture
single_fly_hdf5
.This fixture correctly utilizes the
FileHandle
class to manage the single fly HDF5 file, facilitating its use in tests.
- 52-52: Added constant
TEST_SINGLE_FLY
.This constant serves a similar purpose as
TEST_SINGLE_FLY_H5
, providing a clear reference to the single fly MP4 video file.
- 55-57: Added fixture
single_fly_mp4_vid
.This fixture is well-implemented for loading the single fly MP4 video file using the
Video.from_media
method, aligning with the existing pattern for video fixtures.tests/io/test_formats.py (2)
- 22-22: Import of
SleapAnalysisAdaptor
added.This import is necessary for the new test function
test_sleap_analysis_read
to validate reading Sleap analysis files.
- 25-34: Added test function
test_sleap_analysis_read
.This test function is well-structured and effectively validates the reading of single instance hdf5 analysis files, ensuring that the expected number of videos, tracks, and skeletons are loaded correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the test. For the fixture, I think it's a bit big. Maybe we can use one of the videos we already have (proposing small_robot.mp4) and also making a really small analysis file from that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- tests/fixtures/videos.py (2 hunks)
- tests/io/test_formats.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests/fixtures/videos.py
- tests/io/test_formats.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/fixtures/videos.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/fixtures/videos.py
Description
While performing inference for single instances, there seems to be no track names generated. This is causing
IndexError
while trying to load a HDF5 file since, the number of tracks is 1 and thetrack_names
is empty. So we add a condition to check this and create a single track accordingly.Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
FileHandle
for improved functionality.small_robot_3_frame_hdf5
,single_fly_hdf5
, andsingle_fly_mp4_vid
for testing.TEST_SMALL_ROBOT3_FRAME_H5
,TEST_SINGLE_FLY_H5
, andTEST_SINGLE_FLY
for test scenarios.test_sleap_analysis_read
to read Sleap analysis files usingSleapAnalysisAdaptor
.