Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test InstanceGroup and FrameGroup #1759

Conversation

roomrys
Copy link
Collaborator

@roomrys roomrys commented Apr 29, 2024

Description

This PR adds more tests for InstanceGroup and FrameGroup classes.

Types of changes

  • Bugfix
  • New feature
  • Refactor / Code style update (no logical changes)
  • Build / CI changes
  • Documentation Update
  • Other (tests)

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!

❤️

Copy link

coderabbitai bot commented Apr 29, 2024

Walkthrough

The updates primarily enhance the Camera class by introducing a new parameter to improve data handling, and refining method functionalities. Additionally, the testing suite has been expanded to accommodate new features and ensure robustness, including the introduction of optional parameters and testing for new logic in instance and frame management.

Changes

File Path Change Summary
sleap/io/cameras.py - New invisible_as_nan parameter in numpy method
- Renamed parameter in remove_instance_group
- Placeholder for _remove_camera method
tests/io/test_cameras.py - Updated imports
- Expanded create_instance_group with optional name
- Enhanced tests for instance and frame group handling

Poem

🐇💻🌟
In the land of code and frame,
A little rabbit made its claim.
Tweaks and tests, a dance of sorts,
Ensuring no bug thwarts its forts.
Celebrate, for changes are here,
CodeRabbit hops and spreads cheer!
🎉🐾


Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 507eefa and 3adb85e.
Files selected for processing (2)
  • sleap/io/cameras.py (8 hunks)
  • tests/io/test_cameras.py (10 hunks)
Additional Context Used
Ruff (7)
sleap/io/cameras.py (7)

79-79: f-string without any placeholders


80-80: f-string without any placeholders


81-81: f-string without any placeholders


448-448: Undefined name Skeleton


605-605: Undefined name Skeleton


833-833: Do not use bare except


1007-1007: Undefined name Labels

Additional comments not posted (18)
tests/io/test_cameras.py (16)

15-15: Updated imports to include PredictedInstance.

This change aligns with the new functionalities being tested that involve PredictedInstance.


401-401: Added optional name parameter to create_instance_group.

This addition enhances flexibility by allowing the specification of a name during instance group creation, which is useful for more descriptive testing scenarios.


416-418: Handle default naming within create_instance_group.

Setting a default name when none is provided is a good practice for ensuring that every instance group has a meaningful identifier, especially in testing contexts.


467-471: Test for expected failure when creating a dummy instance.

This test ensures that the method behaves as expected under erroneous conditions, which is crucial for robustness.


490-500: Testing properties of a dummy instance.

These assertions are important for verifying that the dummy instances are correctly initialized with expected default values.


501-507: Testing the name property and its setter.

These tests validate the functionality of getting and setting the name of an instance group, which is important for ensuring data integrity and correct behavior in user interfaces or logs.


508-517: Testing set_name method with name registry.

This test checks the behavior of the set_name method, ensuring it interacts correctly with a name registry to avoid duplicates, which is crucial for maintaining uniqueness constraints.


518-523: Testing generation of a unique name.

This test ensures that the method for generating a unique name functions correctly, which is essential for avoiding conflicts in identifiers within the system.


620-625: Testing the update_points method.

This test verifies that the update_points method correctly updates the points in an instance group, which is crucial for ensuring data consistency after modifications.


626-643: Testing methods for adding, replacing, and removing instances.

These tests are vital for ensuring that instance management within groups is robust and error-free, particularly in scenarios involving complex data manipulations.


747-748: Testing setter for cams_to_include with error handling.

This test ensures that the system correctly handles invalid assignments to cams_to_include, which is important for maintaining the integrity of the data structure.


762-767: Testing retrieval of an instance group based on an instance.

This test checks that the method for retrieving the instance group of a given instance is functioning correctly, which is crucial for navigating complex data relationships within the system.


768-784: Testing the setter and removal methods for instance groups.

These tests ensure that instance groups can be dynamically managed within a frame group, which is essential for applications that require runtime modifications to data structures.


811-823: Testing renaming of instance groups within a frame group.

This test verifies that renaming functionality is properly managed and integrated within the frame group, ensuring consistency and correctness of data.


824-848: Testing methods related to labeled frame management.

These tests are crucial for ensuring that operations related to labeled frames are handled correctly, particularly in contexts where accurate tracking and management of video data are required.


849-855: Testing the creation and addition of labeled frames.

This test ensures that labeled frames can be created and correctly integrated into the system, which is vital for maintaining data integrity and usability in video analysis applications.

sleap/io/cameras.py (2)

Line range hint 554-588: The addition of the invisible_as_nan parameter in the numpy method enhances flexibility in handling invisible points. However, ensure that all calls to this method in the codebase are updated to handle this new parameter.


Line range hint 554-588: The implementation of handling invisible points as NaN in the numpy method is correctly done and enhances data handling.


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

@roomrys roomrys added the MultiView Stack This PR is part of the MultView stacked PRs. label Apr 29, 2024
Copy link

codecov bot commented Apr 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.00%. Comparing base (507eefa) to head (3adb85e).

Additional details and impacted files
@@                                 Coverage Diff                                 @@
##           liezl/asc-initial-update-instances-across-views    #1759      +/-   ##
===================================================================================
+ Coverage                                            73.78%   74.00%   +0.21%     
===================================================================================
  Files                                                  135      135              
  Lines                                                24919    24920       +1     
===================================================================================
+ Hits                                                 18387    18441      +54     
+ Misses                                                6532     6479      -53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@roomrys roomrys marked this pull request as ready for review April 30, 2024 22:54
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (5)
tests/io/test_cameras.py (1)

3-3: Ensure consistent import formatting.

Consider aligning the import statements for better readability. For example, aligning the imported types under each other can improve the visual structure of the import section.

sleap/io/cameras.py (4)

Line range hint 448-448: Undefined name Skeleton used in _create_dummy_instance. Ensure that Skeleton is properly imported or defined in the scope.

+ from sleap.skeleton import Skeleton

Line range hint 605-605: Undefined name Skeleton used in create_and_add_instance. Ensure that Skeleton is properly imported or defined in the scope.

+ from sleap.skeleton import Skeleton

Line range hint 833-833: Avoid using a bare except in __getitem__. Specify the exception type to improve error handling and maintainability.

- except:
+ except IndexError:

Line range hint 79-81: The f-strings in __repr__ methods do not contain placeholders. Consider removing the f-string notation if no formatting is needed.

- f"{session} not found in {self}."
+ "{session} not found in {self}."

@roomrys roomrys merged commit 55c6fd4 into liezl/asc-initial-update-instances-across-views Apr 30, 2024
8 of 9 checks passed
@roomrys roomrys deleted the liezl/test-instance-group-and-frame-group branch May 30, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MultiView Stack This PR is part of the MultView stacked PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant