(3f -> 3e) Add FrameGroup class
#1665
Conversation
WalkthroughThe recent updates introduce significant enhancements to the handling and management of Changes
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Additional comments not posted (5)
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 (
|
Codecov ReportAttention: Patch coverage is
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 ☔ View full report in Codecov by Sentry. |
…almolab/sleap into liezl/add-frame-group
…almolab/sleap into liezl/add-frame-group
…almolab/sleap into liezl/add-frame-group
* Add numpy methods for FrameGroup and InstanceGroup (untested) * Remove commented-out code * Finish using FrameGroup/InstanceGroup for TriangulateSession (untested) * Lint * Modify `TriangulateSession` to be able to triangulate multiple `InstanceGroup`s at once * Lint * Remove obsolete tests
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Actionable comments outside the diff hunks (8)
sleap/io/cameras.py (3)
Line range hint
80-82: Remove unnecessary f-strings as they do not contain any placeholders.- f"No camera has been specified. " + "No camera has been specified. " - f"This is likely because the `Camcorder.from_dict` method was not used to initialize this object. " + "This is likely because the `Camcorder.from_dict` method was not used to initialize this object. " - f"Please use `Camcorder.from_dict` to recreate the object." + "Please use `Camcorder.from_dict` to recreate the object."
Line range hint
412-414: Add missing imports forInstance,Skeleton, andLabeledFrameto resolve undefined names.+ from sleap.instance import Instance, PredictedInstance + from sleap.skeleton import Skeleton + from sleap.labels import LabeledFrameAlso applies to: 427-427, 448-448, 477-477, 504-504, 516-516, 532-532, 561-561, 586-586, 630-630, 650-650, 657-657, 708-708, 721-721, 722-722
Line range hint
743-743: Replace the bareexceptwith a more specific exception handling to avoid catching unexpected exceptions.- except: + except SomeSpecificException:sleap/gui/commands.py (5)
Line range hint
38-40: Remove unused imports to clean up the code.- from itertools import permutations, product - from typing import Callable, Dict, Iterator, List, Optional, Tuple, Type, Union, cast + from typing import Callable, Dict, Iterator, List, Optional, Tuple, Type, Union
Line range hint
196-196: Undefined nameMainWindowused. Ensure thatMainWindowis defined or imported correctly in this context.
Line range hint
838-838: The local variablefile_diris assigned but never used. Consider removing it if it's unnecessary.- file_dir = os.path.dirname(filename)
Line range hint
2470-2470: The f-string does not contain any placeholders. Use a regular string instead.- f"Please select the area from which to remove instances. This will be applied to all frames." + "Please select the area from which to remove instances. This will be applied to all frames."
Line range hint
2802-2802: The f-string does not contain any placeholders. Use a regular string instead.- f"An error occurred while generating suggestions. " + "An error occurred while generating suggestions. "
| @@ -819,8 +821,13 @@ class RecordingSession: | |||
| metadata: dict = field(factory=dict) | |||
| _video_by_camcorder: Dict[Camcorder, Video] = field(factory=dict) | |||
| labels: Optional["Labels"] = None | |||
There was a problem hiding this comment.
Add missing import for Labels to resolve the undefined name.
+ from sleap.labels import LabelsCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| labels: Optional["Labels"] = None | |
| from sleap.labels import Labels | |
| labels: Optional["Labels"] = None |
| # "Hidden" class attribute | ||
| _cams_to_include: Optional[List[Camcorder]] = None | ||
| _excluded_views: Optional[Tuple[str]] = () | ||
| _dummy_labeled_frame: Optional["LabeledFrame"] = None |
There was a problem hiding this comment.
Add missing imports for Instance, Skeleton, and LabeledFrame to resolve undefined names.
+ from sleap.instance import Instance, PredictedInstance
+ from sleap.skeleton import Skeleton
+ from sleap.labels import LabeledFrameAlso applies to: 1265-1265, 1266-1266, 1271-1271, 1342-1342, 1354-1354, 1406-1406, 1493-1493, 1514-1514, 1530-1530, 1542-1542, 1574-1574, 1678-1678, 1694-1694, 1780-1780, 1801-1801, 1802-1802, 1805-1805, 1862-1862, 1876-1876, 1914-1914, 1918-1918, 1924-1924, 1925-1925, 1926-1926, 1935-1935, 1971-1971, 1976-1976, 1980-1980
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| _dummy_labeled_frame: Optional["LabeledFrame"] = None | |
| from sleap.labels import LabeledFrame | |
| _dummy_labeled_frame: Optional["LabeledFrame"] = None |
* Add method to get single instance permutations * Add method and (failing) test to get instance grouping * Append a dummy instance for missing instances * Update tests to accept a dummy instance * Add initial InstanceGroup class * Few extra tests for `InstanceGroup` * Remember instance grouping after testing hypotheses * Use reconsumable iterator for reprojected coords * Only triangulate user instances, add fixture, update tests * Normalize instance reprojection errors * Add `locked`, `_dummy_instance`, `numpy`, and `update_points` * Allow `PredictedPoint`s to be updated as well * Add tests for new attributes and methods * Add methods to create, add, replace, and remove instances * Use PredictedInstance for new/dummy instances * (3f -> 3e) Add `FrameGroup` class (#1665) * (3g -> 3f) Use frame group for triangulation (#1693)
* Update methods to allow triangulating multiple instances at once * Return instances and coords as a dictionary with cams * Update get_instance_across_views to handle multiple frames * [wip] Update calculate reprojected points to support multiple frames * Finish support for multi-frame reprojection * Remove code to put in next PR * (3b -> 3a) Add method to get single instance permutations (#1586) * Add method to get single instance permutations * Append a dummy instance for missing instances * Correct 'permutations' to 'products' * (3c -> 3b) Add method to test instance grouping (#1599) * (3d -> 3c) Add method for multi instance products (#1605) * (3e -> 3a) Add `InstanceGroup` class (#1618) * Add method to get single instance permutations * Add method and (failing) test to get instance grouping * Append a dummy instance for missing instances * Update tests to accept a dummy instance * Add initial InstanceGroup class * Few extra tests for `InstanceGroup` * Remember instance grouping after testing hypotheses * Use reconsumable iterator for reprojected coords * Only triangulate user instances, add fixture, update tests * Normalize instance reprojection errors * Add `locked`, `_dummy_instance`, `numpy`, and `update_points` * Allow `PredictedPoint`s to be updated as well * Add tests for new attributes and methods * Add methods to create, add, replace, and remove instances * Use PredictedInstance for new/dummy instances * (3f -> 3e) Add `FrameGroup` class (#1665) * (3g -> 3f) Use frame group for triangulation (#1693) * Only use user-`Instance`s for triangulation * Remove unused code * Use `LabeledFrame` class instead of dummy labeled frame * Limit which methods can update `Labels.labeled_frames` * Update cache when Labels. remove_session_video * Remove RecordingSession.instance_groups * [wip] Maintain cached FrameGroup dictionaries * Add unique name (per FrameGroup) requirement for InstanceGroup * Lint * Fix remove_video bug * Add RecordingSession.new_frame_group method * Add TODO comments for later * Fix RecordingSesssion.remove_video bug * Remove FrameGroup._frame_idx_registry class attribute
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)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
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit