coderabbitai[bot]
commented
8 months ago Walkthrough
The recent updates introduce significant enhancements to the handling and management of FrameGroup objects in both the GUI and I/O components of the software. Key modifications include the transition from FrameGroup to InstanceGroup in the GUI, along with expanded functionalities in the cameras.py module to better manage instances and labeled frames across different views.
Changes
| Files | Change Summary |
|---|---|
sleap/gui/commands.pysleap/io/cameras.py |
Updated FrameGroup to InstanceGroup in GUI commands and added new functionalities for managing FrameGroup objects in I/O cameras. |
tests/gui/test_commands.pytests/io/test_cameras.py |
Removed outdated tests in GUI related to triangulation and updated tests in I/O to cover new FrameGroup functionalities. |
Poem
🐇🌟
In the land of code where the data hops,
A change was made at the treetops.
FrameGroupdances, nowInstanceGroupsings,
Across the wires, data flutters its wings.
Cheers to the bytes, in harmony they mesh,
For in this update, our systems refresh.
🌼🐇
Recent Review Details
**Configuration used: CodeRabbit UI**Commits
Files that changed from the base of the PR and between 95071c1c20bf6d57b7c0995ceb12d40ca804db9b and ff0924e75d1145a276a8f3b74dbf08e07afbf8a6.Files selected for processing (4)
* sleap/gui/commands.py (2 hunks) * sleap/io/cameras.py (10 hunks) * tests/gui/test_commands.py (1 hunks) * tests/io/test_cameras.py (4 hunks)Additional comments not posted (5)
tests/io/test_cameras.py (1)
`418-447`: The tests for `FrameGroup` correctly verify the creation from instance groups and the management of frame indices. Ensure that the shared registry of frame indices behaves as expected in concurrent scenarios.Verification successful
--- The search results from the script provide insights into the usage of `_frame_idx_registry` within the codebase. The references found in `tests/io/test_cameras.py` and `sleap/io/cameras.py` indicate that `_frame_idx_registry` is used to track frame indices across different instances of `FrameGroup` and within the `RecordingSession` context. From the output: 1. In `test_cameras.py`, the tests verify the behavior of `_frame_idx_registry` by asserting its contents after creating `FrameGroup` instances. This confirms that the test checks the shared registry behavior as intended. 2. In `cameras.py`, `_frame_idx_registry` is initialized as an empty dictionary and is used to add frame indices as they are processed. This suggests that the class handles the addition of new frame indices and ensures that they are tracked correctly across sessions. Given this information, the implementation and testing of `_frame_idx_registry` appear to be consistent with the expectations set in the review comment. The tests in `test_cameras.py` are aligned with the implementation in `cameras.py`, verifying the correct behavior of the frame index registry.Scripts Executed
The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Search for any concurrent modifications to `_frame_idx_registry` across the codebase. rg --type python '_frame_idx_registry' --context 5 ``` Length of output: 81 --- --- Script: ```shell #!/bin/bash # Corrected search for any concurrent modifications to `_frame_idx_registry` across the codebase. rg '_frame_idx_registry' --type py --context 5 ``` Length of output: 3968tests/gui/test_commands.py (1)
Line range hint `227-299`: The tests for `ExportAnalysisFile` cover various scenarios effectively, including different file formats and conditions with no labeled frames. Ensure that the export functionality handles edge cases as expected.Verification successful
It appears that the previous script failed due to an unrecognized file type specification for `rg`. The correct usage should not specify the file type as `python` since `rg` does not recognize it by default. Instead, we should use a more general approach or specify the file extensions directly. Let's correct this and rerun the script to ensure we can verify the usage of `ExportAnalysisFile` across the codebase effectively. --- The search results confirm the usage of `ExportAnalysisFile` within the `tests/gui/test_commands.py` and `sleap/gui/commands.py` files. The test cases in `test_commands.py` extensively use the `ExportAnalysisFile` class to test various scenarios, including exporting with different parameters and handling different file formats. The class is also referenced in `sleap/gui/commands.py` where it is used to execute export operations with specific parameters. From the output, it is evident that the `ExportAnalysisFile` class is integral to the functionality of exporting analysis files in different formats and is well-integrated into the GUI command structure. The tests cover a wide range of scenarios ensuring that the functionality behaves as expected under various conditions. Given this comprehensive integration and testing, the changes in the `test_ExportAnalysisFile` function and the usage of `ExportAnalysisFile` across the codebase are verified to be consistent and correctly implemented.Scripts Executed
The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Search for any additional usage of `ExportAnalysisFile` that might affect its behavior in edge cases. rg --type python 'ExportAnalysisFile' --context 5 ``` Length of output: 80 --- --- Script: ```shell #!/bin/bash # Search for any additional usage of `ExportAnalysisFile` that might affect its behavior in edge cases. rg 'ExportAnalysisFile' --context 5 --type py ``` Length of output: 12138sleap/gui/commands.py (3)
Line range hint `1712-1712`: Avoid using bare `except` statements. Specify the exception types to handle specific errors appropriately. --- Line range hint `1733-1733`: Avoid using bare `except` statements. It's a good practice to specify the exceptions you are catching. --- Line range hint `3145-3145`: Avoid using bare `except` statements. It's a good practice to specify the exceptions you are catching.
codecov[bot]
Description
This PR adds the
FrameGroupclass which is akin to aLabeledFramebut across all views in aRecordingSession. The tasks to do here are:FrameGroupclassRecordingSessionFrameGroupFrameGroupFrameGrouphypotheses generation methodAdd
FrameGroupclassFrameGroupto theRecordingSessionthrough aFrameGroup.sessioninstance attributeFrameGroup.frame_idxinstance attribute_frame_idx_registryclass attribute to ensure theFrameGroupis unique perRecordingSessionandframe_idxframe_idxandsessionare unique to thisFrameGroupupon initializationInstanceGroups in the currentFrameGroupviaFrameGroup.instance_groups_labeled_frames_by_cam: Dict[Camcorder, LabeledFrame]_labeled_frames_by_cameach time aLabeledFrameis added/removed from theFrameGroup.session.labels: Labelsobject_labeled_frames_by_cameach time aVideois added/removed from theFrameGroup.session: RecordingSessionobject_labeled_frames_by_camto return a list ofLabeledFrames in thelabeled_framesproperty_labeled_frames_by_camto return a list ofCamcordersin thecameraspropertyFrameGroupfrom aList[InstanceGroup](andRecordingSessionandframe_idx)FrameGroupfrom aDict[Camcorder, List[Instance]](andRecordingSessionandframe_idx)Integrate with
RecordingSessionRecordingSession._frame_group_by_frame_idx: Dict[int, FrameGroup]attribute to store all knownFrameGroups by theirframe_idx(also used to ensure uniqueness ofFrameGroups)RecordingSesssion._frame_group_by_frame_idxin aRecordingSession.frame_group -> Dict[int, FrameGroup]propertyRecordingSesssion._frame_group_by_frame_idxin aRecordingSession.frame_inds -> List[int]propertyfrom_session_dictmethod to reconstructFrameGroupsto_session_dictmethod to write aFrameGroupto an slpSerialize/Deserialize
FrameGroupFrameGroup.make_cattrmethodLabeledFrame.instancesRecordingSessionsupdate_locked_instances_by_camAdd a hypothesis generation with
FrameGroup.generate_hypothesesInstanceGroups are locked (or set by the user) inFrameGroup._locked_instance_groups: List[InstanceGroup]FrameGroup._locked_instance_groupswhenever anInstanceGroupis added/removed/locked/unlockedCamcorders to include in theFrameGroupas a class attributeFrameGroup._cams_to_include: Optional[List[Camcorder]]_cams_to_includein thecams_to_includeproperty to return either the list ofCamcorders in_cams_to_includeor the list ofCamcorders inFrameGroup.session.camera_cluster.cameras_instances_by_cam: Dict[Camcorder, Set[Instance]]as an instance attribute_instances_by_camsimilarly to_labeled_frames_by_cam_instances_by_camin theinstances_by_cams_to_includeproperty to only returnDict[Camcorder, Set[Instance]]from thecams_to_includegenerate_hypothesesmethod which usesinstances_by_cams_to_includeto gather all unlocked instances across views, fill in the "missing" instances in each view, permute all unlocked instances, take products of unlocked instances, and reorganize products intogrouping_hypotheses: Dict[frame_id: int, List[InstanceGroup]]ORgrouping_hypotheses: Dict[frame_id: int, Dict[Camcorder, List[Instance]]]Use
FrameGrouphypotheses generation methodTriangulateSessionwith thesession.frame_group[frame_idx].generate_hypotheses()TriangulateSessionuses the "cached" reprojections from all hypotheses to just update theInstances inTriangulateSession.do_action(), but we also need a way to trangulate, reproject, and updateInstances without doing hypothesis testingOthers (not needed for this PR, but got distracted and added anyway)
RecordingSession.linked_camerass.t. the ordering followsRecordingSession.camerasRecordingSession.unlinked_camerass.t. the ordering followsRecordingSession.camerasRecordingSession.camera_cluster._videos_by_session[RecordingSession]s.t. the ordering followsRecordingSession.cameras(this is useful when navigating through views in the GUI)Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
:heart:
Summary by CodeRabbit