coderabbitai[bot]
commented
1 year ago Walkthrough
The recent changes across the codebase focused on enhancing the AddInstances class in sleap/gui/commands.py. These updates include method renaming, parameter adjustments, and return type refinements. Additionally, related test functions in tests/gui/test_commands.py have been modified to align with these class changes.
Changes
| File Path | Summary |
|---|---|
sleap/gui/commands.py |
Various updates to the AddInstances class, including method renaming, parameter changes, and return type enhancements. The typing import statement has also been revised. |
sleap/gui/dialogs/delete.py |
Logic change in the _delete method to call remove_frame instead of remove when no instances are left in a labeled frame. |
sleap/instance.py |
Updates in handling points within the update_points method, skipping NaN values, extracting attributes, and updating the points dictionary. |
sleap/io/dataset.py |
Additions like a new method remove_session_video, updated docstrings, and TODO comments for handling LabeledFrame removal in various methods. |
tests/fixtures/datasets.py |
Addition of multiview_min_session_user_labels fixture for loading user-labeled data in a minimal session. |
tests/gui/test_commands.py |
Removal of test functions related to triangulating sessions and verifying instances across views. |
tests/io/test_cameras.py |
Modifications in declarations and tests for InstanceGroup and FrameGroup entities. |
tests/io/test_dataset.py |
Modification in calling remove_session_video with explicit parameter naming. |
Poem
🐇 Code changes dance like leaves in the wind, 🍃
Bringing clarity, a new path to find. 🛤️
Methods refactored, tests now aligned, 🛠️
In the realm of code, improvements designed. 🌌
A tale of updates, in lines and in tests, 📜
Progress whispers in the coder's quests. 🌠
Recent Review Details
**Configuration used: CodeRabbit UI**Commits
Files that changed from the base of the PR and between d83ea6457d4ee75caa4ee8549564040ae8db0f7d and 176efb26e30961980ddcb076c73032a14e8bba85.Files selected for processing (9)
* sleap/gui/commands.py (4 hunks) * sleap/gui/dialogs/delete.py (1 hunks) * sleap/instance.py (2 hunks) * sleap/io/cameras.py (8 hunks) * sleap/io/dataset.py (10 hunks) * tests/fixtures/datasets.py (1 hunks) * tests/gui/test_commands.py (2 hunks) * tests/io/test_cameras.py (3 hunks) * tests/io/test_dataset.py (1 hunks)Files not summarized due to errors (1)
* sleap/io/cameras.py: Error: Message exceeds token limitAdditional Context Used
Ruff (64)
sleap/gui/commands.py (10)
38-38: `itertools.permutations` imported but unused --- 38-38: `itertools.product` imported but unused --- 40-40: `typing.cast` imported but unused --- 196-196: Undefined name `MainWindow` --- 838-838: Local variable `file_dir` is assigned to but never used --- 1712-1712: Do not use bare `except` --- 1733-1733: Do not use bare `except` --- 2470-2470: f-string without any placeholders --- 2802-2802: f-string without any placeholders --- 3145-3145: Do not use bare `except`sleap/instance.py (6)
89-89: Undefined name `self` --- 89-89: Undefined name `self` --- 411-411: Do not compare types, use `isinstance()` --- 449-449: Do not compare types, use `isinstance()` --- 503-503: Do not use bare `except` --- 1652-1652: Undefined name `Labels`sleap/io/cameras.py (8)
6-6: `typing.cast` imported but unused --- 79-79: f-string without any placeholders --- 80-80: f-string without any placeholders --- 81-81: f-string without any placeholders --- 449-449: Undefined name `Skeleton` --- 602-602: Undefined name `Skeleton` --- 829-829: Do not use bare `except` --- 917-917: Undefined name `Labels`sleap/io/dataset.py (16)
55-55: Redefinition of unused `Callable` from line 45 --- 61-61: `h5py` imported but unused --- 69-69: Do not use bare `except` --- 70-70: `typing._ForwardRef` imported but unused --- 365-365: Do not assign a `lambda` expression, use a `def` --- 367-370: Do not assign a `lambda` expression, use a `def` --- 372-375: Do not assign a `lambda` expression, use a `def` --- 934-934: Do not compare types, use `isinstance()` --- 1454-1454: Do not compare types, use `isinstance()` --- 2355-2355: Undefined name `glob` --- 2450-2450: Avoid equality comparisons to `False`; use `if not ret:` for false checks --- 2538-2538: Avoid equality comparisons to `False`; use `if not ret:` for false checks --- 2553-2553: Undefined name `sleap` --- 2643-2643: Do not compare types, use `isinstance()` --- 2646-2646: Local variable `e` is assigned to but never used --- 2648-2648: f-string without any placeholderstests/fixtures/datasets.py (1)
1-1: `os` imported but unusedtests/gui/test_commands.py (17)
5-5: `typing.Dict` imported but unused --- 7-7: `numpy` imported but unused --- 21-21: `sleap.gui.commands.TriangulateSession` imported but unused --- 25-25: `sleap.io.cameras.Camcorder` imported but unused --- 75-75: Ambiguous variable name: `l` --- 82-82: Ambiguous variable name: `l` --- 227-227: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 236-236: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 244-244: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 253-253: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 265-265: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 274-274: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 281-281: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 299-299: Avoid equality comparisons to `True`; use `if okay:` for truth checks --- 360-360: Local variable `last_lf_frame` is assigned to but never used --- 368-368: Avoid equality comparisons to `True`; use `if video.backend.grayscale:` for truth checks --- 528-528: f-string without any placeholderstests/io/test_dataset.py (6)
119-119: Test for object identity should be `is not` --- 120-120: Test for membership should be `not in` --- 121-121: Test for object identity should be `is not` --- 692-692: Local variable `new_labels_json` is assigned to but never used --- 708-708: Local variable `new_labels_json` is assigned to but never used --- 1495-1495: Local variable `track_insts` is assigned to but never used
Learnings (1)
sleap/io/dataset.py (1)
``` User: roomrys" PR: talmolab/sleap#1654 File: sleap/io/dataset.py:1717-1725 Timestamp: 2024-03-15T23:59:34.765Z Learning: When removing a recording session using the `remove_recording_session` method in the `Labels` class, it's necessary to also update the `LabelsDataCache._session_by_video` dictionary to reflect the removal. This ensures that the mapping between videos and their corresponding recording sessions remains accurate. ```
Additional comments not posted (12)
sleap/gui/dialogs/delete.py (1)
`219-219`: Ensure proper handling when no instances are left in a labeled frame. This change optimizes the removal process by directly removing the frame when it's empty, which is a logical improvement. Please ensure that `remove_frame` handles all necessary cleanup and state updates.tests/fixtures/datasets.py (1)
`283-288`: New fixture for loading user-labeled data in a minimal session. This fixture correctly loads the specified dataset and follows the established pattern for dataset fixtures in this file. Good job on maintaining consistency.tests/io/test_cameras.py (2)
`299-348`: New function to create `InstanceGroup` objects. This function is well-implemented and provides flexibility with the `add_dummy` parameter to test different scenarios. Ensure comprehensive tests cover all paths, including error handling and edge cases. --- `173-179`: New method to create `FrameGroup` objects. The method `new_frame_group` is correctly implemented and integrates well with the existing `RecordingSession` structure. Good use of assertions to validate the behavior.tests/io/test_dataset.py (1)
`1033-1033`: Explicitly naming the `video` parameter in the function call enhances readability and reduces potential errors related to positional arguments. Good practice!sleap/instance.py (1)
`732-754`: Consider handling NaN values more robustly in `update_points`. The current implementation skips updating points if the new point is NaN or the old point is marked as complete. While this is generally a good approach, consider adding a mechanism to handle unexpected NaN values more gracefully, perhaps by logging or raising an exception if NaN values are not expected in this context.sleap/io/dataset.py (1)
Line range hint `365-365`: Avoid assigning lambda expressions directly to variables. Consider defining a function with `def` instead of using a lambda expression for better readability and debuggability.sleap/gui/commands.py (5)
Line range hint `1712-1712`: Avoid using bare `except` statements as they can catch unexpected exceptions and hide bugs. Consider specifying the exception types that you expect to handle. --- Line range hint `1733-1733`: Avoid using bare `except` statements as they can catch unexpected exceptions and hide bugs. Consider specifying the exception types that you expect to handle. --- Line range hint `2470-2470`: The f-string does not contain any placeholders, which makes the use of f-string unnecessary. Consider using a regular string if no formatting is needed. --- Line range hint `2802-2802`: The f-string does not contain any placeholders, which makes the use of f-string unnecessary. Consider using a regular string if no formatting is needed. --- Line range hint `3145-3145`: Avoid using bare `except` statements as they can catch unexpected exceptions and hide bugs. Consider specifying the exception types that you expect to handle.
codecov[bot]
roomrys
Description
This PR aims to add a method to match instances across views (without knowing
Tracks).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
delete.pyfile for improved frame removal handling.instance.pyfile to improve point object creation.dataset.pyfile for better video and session management.datasets.pyfile.test_commands.pyfile to reflect changes in session handling.test_cameras.pyfile to align with entity imports.test_dataset.pyfile for explicit parameter passing.