(3f -> 3e) Add `FrameGroup` class

[roomrys]

Description

This PR adds the FrameGroup class which is akin to a LabeledFrame but across all views in a RecordingSession. The tasks to do here are:

• [ ] Add FrameGroup class
• [ ] Integrate with RecordingSession
• [ ] Serialize/Deserialize FrameGroup
• [ ] Add a hypotheses generation method to FrameGroup
• [ ] Use FrameGroup hypotheses generation method

---

Add FrameGroup class

• [x] Link the FrameGroup to the RecordingSession through a FrameGroup.session instance attribute
• [x] Keep tabs on the which frame index is being used through FrameGroup.frame_idx instance attribute
• [x] Add _frame_idx_registry class attribute to ensure the FrameGroup is unique per RecordingSession and frame_idx
• [x] Enforce that frame_idx and session are unique to this FrameGroup upon initialization
• [x] Keep a list of InstanceGroups in the current FrameGroup via FrameGroup.instance_groups
• [x] Keep a dictionary mapping _labeled_frames_by_cam: Dict[Camcorder, LabeledFrame]
• [ ] Maintain _labeled_frames_by_cam each time a LabeledFrame is added/removed from the FrameGroup.session.labels: Labels object
• [ ] Maintain _labeled_frames_by_cam each time a Video is added/removed from the FrameGroup.session: RecordingSession object
• [x] Use _labeled_frames_by_cam to return a list of LabeledFrames in the labeled_frames property
• [x] Use _labeled_frames_by_cam to return a list of Camcorders in the cameras property
• [x] Add a class method to create a FrameGroup from a List[InstanceGroup] (and RecordingSession and frame_idx)
• [ ] Add a class method to create a FrameGroup from a Dict[Camcorder, List[Instance]] (and RecordingSession and frame_idx)

Integrate with RecordingSession

• [x] Add a RecordingSession._frame_group_by_frame_idx: Dict[int, FrameGroup] attribute to store all known FrameGroups by their frame_idx (also used to ensure uniqueness of FrameGroups)
• [x] Use RecordingSesssion._frame_group_by_frame_idx in a RecordingSession.frame_group -> Dict[int, FrameGroup] property
• [x] Use RecordingSesssion._frame_group_by_frame_idx in a RecordingSession.frame_inds -> List[int] property
• [ ] Add to the from_session_dict method to reconstruct FrameGroups
• [ ] Add to the to_session_dict method to write a FrameGroup to an slp

Serialize/Deserialize FrameGroup

• [ ] Add FrameGroup.make_cattr method

Attribute Name	Type	Description
frame_idx	int	Instance attribute
instance_groups	List[InstanceGroup]	Instance attribute, akin to LabeledFrame.instances
session	RecordingSession	Instance attribute
_frame_idx_registry	Dict[RecordingSession, Set[int]]	Class attribute to keep track of frame indices across all RecordingSessions
_cams_to_include	Optional[List[Camcorder]]	"Hidden" class attribute
_excluded_views	Optional[Tuple[str]]	"Hidden" class attribute
_labeled_frames_by_cam	Dict[Camcorder, "LabeledFrame"]	"Hidden" instance attribute, should be updated each time a LabeledFrame is added/removed from the Labels object or if a video is added/removed from the RecordingSession
_instances_by_cam	Dict[Camcorder, Set["Instance"]]	"Hidden" instance attribute
_locked_instance_groups	List[InstanceGroup]	"Hidden" instance attribute, should be updated each time an InstanceGroup is added/removed/locked/unlocked
_locked_instances_by_cam	Dict[Camcorder, Set["Instance"]]	"Hidden" instance attribute, internally updated in update_locked_instances_by_cam

Add a hypothesis generation with FrameGroup.generate_hypotheses

• [x] Keep track of which InstanceGroups are locked (or set by the user) in FrameGroup._locked_instance_groups: List[InstanceGroup]
• [ ] Maintain FrameGroup._locked_instance_groups whenever an InstanceGroup is added/removed/locked/unlocked
• [x] Keep track of which Camcorders to include in the FrameGroup as a class attribute FrameGroup._cams_to_include: Optional[List[Camcorder]]
• [x] Use _cams_to_include in the cams_to_include property to return either the list of Camcorders in _cams_to_include or the list of Camcorders in FrameGroup.session.camera_cluster.cameras
• [x] Keep track of the _instances_by_cam: Dict[Camcorder, Set[Instance]] as an instance attribute
• [ ] Maintain _instances_by_cam similarly to _labeled_frames_by_cam
• [x] Use _instances_by_cam in the instances_by_cams_to_include property to only return Dict[Camcorder, Set[Instance]] from the cams_to_include
• [x] Add a generate_hypotheses method which uses instances_by_cams_to_include to 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 into grouping_hypotheses: Dict[frame_id: int, List[InstanceGroup]] OR grouping_hypotheses: Dict[frame_id: int, Dict[Camcorder, List[Instance]]]

Use FrameGroup hypotheses generation method

• [ ] Swap out the hypotheses generation in TriangulateSession with the session.frame_group[frame_idx].generate_hypotheses()
• [ ] Currently TriangulateSession uses the "cached" reprojections from all hypotheses to just update the Instances in TriangulateSession.do_action(), but we also need a way to trangulate, reproject, and update Instances without doing hypothesis testing

Others (not needed for this PR, but got distracted and added anyway)

• [x] Sort the RecordingSession.linked_cameras s.t. the ordering follows RecordingSession.cameras
• [x] Sort the RecordingSession.unlinked_cameras s.t. the ordering follows RecordingSession.cameras
• [x] Sort the RecordingSession.camera_cluster._videos_by_session[RecordingSession] s.t. the ordering follows RecordingSession.cameras (this is useful when navigating through views in the GUI)

Fig 1: Three different flavors of the flow chart plan with the middle plan being the most complete, but the top being the easiest (the bottom is just unneeded work).

Types of changes

• [ ] Bugfix
• [x] New feature
• [ ] Refactor / Code style update (no logical changes)
• [ ] Build / CI changes
• [ ] Documentation Update
• [ ] Other (explain)

Does this address any currently open issues?

[list open issues here]

Outside contributors checklist

• [ ] Review the guidelines for contributing to this repository
• [ ] Read and sign the CLA and add yourself to the authors list
• [ ] Make sure you are making a pull request against the develop branch (not main). Also you should start your branch off develop
• [ ] Add tests that prove your fix is effective or that your feature works
• [ ] Add necessary documentation (if appropriate)

Thank you for contributing to SLEAP!

:heart:

[coderabbitai[bot]]

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.py sleap/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.py tests/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.
FrameGroup dances, now InstanceGroup sings,
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:

#!/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:

#!/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: 3968

tests/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:

#!/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:

#!/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: 12138

sleap/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.

---

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 46.15385% with 133 lines in your changes are missing coverage. Please review.

Project coverage is 73.60%. Comparing base (95071c1) to head (a99cef5).

:exclamation: Current head a99cef5 differs from pull request most recent head ff0924e. Consider uploading reports for the commit ff0924e to get more accurate results

Files	Patch %	Lines
sleap/io/cameras.py	46.15%	133 Missing :warning:

Additional details and impacted files

@@                        Coverage Diff                         @@
##           liezl/add-instance-group-class    #1665      +/-   ##
==================================================================
- Coverage                           73.85%   73.60%   -0.26%     
==================================================================
  Files                                 135      135              
  Lines                               24807    25037     +230     
==================================================================
+ Hits                                18322    18428     +106     
- Misses                               6485     6609     +124     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.