(4->3) Fix (de)serialization of RecordingSession and add Multiview Fixture
#1281
Conversation
Codecov Report
@@ Coverage Diff @@
## liezl/asc-initial-update-instances-across-views #1281 +/- ##
==================================================================================
Coverage ? 72.82%
==================================================================================
Files ? 134
Lines ? 24101
Branches ? 0
==================================================================================
Hits ? 17551
Misses ? 6550
Partials ? 0 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…iezl/iuiav-fix-(de)-serialization
…ps://github.com/talmolab/sleap into liezl/iuiav-fix-(de)-serialization
…ps://github.com/talmolab/sleap into liezl/iuiav-fix-(de)-serialization
roomrys
changed the title
RecordingSessionRecordingSession and add Multiview Fixture
roomrys
changed the title
RecordingSession and add Multiview FixtureRecordingSession and add Multiview Fixture
…iezl/iuiav-fix-(de)-serialization
…ps://github.com/talmolab/sleap into liezl/iuiav-fix-(de)-serialization
WalkthroughThis update enhances the handling of sessions in SLEAP, including better representation of camera and video data, improved session key management, and expanded testing capabilities. It also includes minor comment corrections. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files ignored due to filter (9)
- tests/data/cameras/minimal_session/calibration.toml
- tests/data/videos/min_session_back.mp4
- tests/data/videos/min_session_backL.mp4
- tests/data/videos/min_session_mid.mp4
- tests/data/videos/min_session_midL.mp4
- tests/data/videos/min_session_side.mp4
- tests/data/videos/min_session_sideL.mp4
- tests/data/videos/min_session_top.mp4
- tests/data/videos/min_session_topL.mp4
Files selected for processing (6)
- sleap/io/cameras.py (3 hunks)
- sleap/io/dataset.py (1 hunks)
- sleap/io/format/hdf5.py (3 hunks)
- tests/fixtures/datasets.py (1 hunks)
- tests/io/test_cameras.py (3 hunks)
- tests/io/test_dataset.py (2 hunks)
Files skipped from review due to trivial changes (1)
- sleap/io/dataset.py
Additional comments (Suppressed): 10
tests/fixtures/datasets.py (1)
- 275-280: The new fixture
multiview_min_session_labelsloads a minimal session file and searches for videos in the specified directory. Ensure that the path to the.slpfile and the video directory are correct and accessible. Also, verify that the loaded labels are used correctly in the tests where this fixture is applied.tests/io/test_cameras.py (2)
66-66: The number of cameras in the camera cluster has changed from 4 to 8. Please verify if this change is intentional and aligns with the test data.
204-209: The keys of
session_dict["camcorder_to_video_idx_map"]have been changed from integers to strings. This could potentially break other parts of the code that expect these keys to be integers. Please ensure that all references to these keys are updated accordingly.sleap/io/format/hdf5.py (3)
86-91: The "sessions" key has been added to the loop for deserializing metadata from the HDF5 file. Ensure that all existing files that this function will be used on contain the "sessions" key or can handle its absence gracefully.
325-330: The "sessions" key has been added to the loop for serializing metadata into the HDF5 file. This change should be compatible with the deserialization changes made in lines 86-91.
343-346: There's a typo correction in the comment, changing "PredcitedInstances" to "PredictedInstances". This doesn't affect the code execution but improves readability.
sleap/io/cameras.py (3)
762-766: The
__repr__method has been updated to include the number of videos in the representation. This change provides more detailed information about theRecordingSessionobject, which can be useful for debugging and logging.828-834: The keys of
camcorder_to_video_idx_mapare now being converted to strings before assignment. This change seems to be related to serialization/deserialization, as JSON objects only support string keys. However, this could potentially introduce type-related bugs elsewhere in the code if it's expected that these keys are integers. Please verify that all uses ofcamcorder_to_video_idx_mapthroughout the codebase have been updated to handle string keys.- camcorder_to_video_idx_map[cam_idx] = video_idx + camcorder_to_video_idx_map[str(cam_idx)] = str(video_idx)
- 864-869: Similar to the previous comment, the keys of
camcorder_to_video_idx_mapare now being converted back to integers when retrievingCamcorderandVideoobjects. Again, please verify that all uses ofcamcorder_to_video_idx_mapthroughout the codebase have been updated to handle this change.- camcorder = session.camera_cluster.cameras[cam_idx] - video = videos_list[video_idx] + camcorder = session.camera_cluster.cameras[int(cam_idx)] + video = videos_list[int(video_idx)]tests/io/test_dataset.py (1)
- 972-979: The function signature of
test_save_labels_with_sessionshas been updated to include a new parametermultiview_min_session_labels. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
roomrys
merged commit ba2df88
into
liezl/asc-initial-update-instances-across-views
Description
This PR adds new test dataset which also revealed some errors in the (de)serialization of
RecordingSesssion. Hence, this PR also fixes those errors (and serves to stand as an entry-point for any future (de)serialization fixes for the lifetime of the multi-view mvp stacked prs).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
__repr__method incameras.pyanddataset.pyto provide more detailed information about videos, camera clusters, and sessions.to_session_dictandfrom_session_dictmethods incameras.py, ensuring proper conversion between integer and string indices.read_headersandappend_uniquefunctions inhdf5.pyto include the "sessions" key for improved data organization.multiview_min_session_labelsindatasets.pyfor comprehensive testing. Adjusted test cases intest_cameras.pyandtest_dataset.pyto align with the latest changes.hdf5.py.