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

Refactor AddInstance command #1561

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Refactor AddInstance command #1561

merged 3 commits into from
Oct 19, 2023

Conversation

roomrys
Copy link
Collaborator

@roomrys roomrys commented Oct 19, 2023

Description

The AddInstance command has a long and confusing do_action method. This PR makes no logical changes and just organizes the do_action method into its subparts.

Types of changes

  • Bugfix
  • 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!

❤️

Summary by CodeRabbit

  • Refactor: The "Add Instance" feature in the SLEAP GUI has been improved for better performance and reliability. This update enhances the process of creating a new instance and filling missing nodes, making it more efficient and user-friendly. The changes are not directly visible to the end-user but contribute to a smoother, more responsive user experience.

@roomrys roomrys marked this pull request as ready for review October 19, 2023 13:41
@coderabbitai
Copy link

coderabbitai bot commented Oct 19, 2023

Walkthrough

The changes primarily focus on the sleap/gui/commands.py file, enhancing its readability and maintainability. The AddInstance class has been refactored, with several methods extracted for better modularity. Unused imports and unnecessary empty lines have been removed, contributing to a cleaner and more organized codebase.

Changes

File Summary
.../gui/commands.py Unused imports and empty lines have been removed. The AddInstance class has been refactored for improved readability and maintainability. Several methods have been extracted for better modularity, and the get_previous_frame_index method has been moved to the AddInstance class.

🐇

"In the land of code, where the shadows lie,

Refactoring reigns, under the cloudless sky.

Unused imports banished, empty lines no more,

The rabbit hops along, leaving folklore.

With every hop and skip, the code becomes clear,

Celebrate the changes, for the end is near!" 🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

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.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 1e0627a and 5e61f00.
Files selected for processing (1)
  • sleap/gui/commands.py (17 hunks)
Files skipped from review due to trivial changes (1)
  • sleap/gui/commands.py

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.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 5e61f00 and 7751925.
Files selected for processing (1)
  • sleap/gui/commands.py (17 hunks)
Files skipped from review due to trivial changes (1)
  • sleap/gui/commands.py

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.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 7751925 and 598da8e.
Files selected for processing (1)
  • sleap/gui/commands.py (17 hunks)
Files skipped from review due to trivial changes (1)
  • sleap/gui/commands.py

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #1561 (598da8e) into develop (1e0627a) will increase coverage by 0.00%.
The diff coverage is 91.30%.

@@           Coverage Diff            @@
##           develop    #1561   +/-   ##
========================================
  Coverage    73.37%   73.37%           
========================================
  Files          134      134           
  Lines        23976    23992   +16     
========================================
+ Hits         17592    17604   +12     
- Misses        6384     6388    +4     
Files Coverage Δ
sleap/gui/commands.py 61.67% <91.30%> (+0.32%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@roomrys roomrys requested a review from talmo October 19, 2023 15:05
Copy link
Collaborator

@talmo talmo left a comment

Choose a reason for hiding this comment

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

Much cleaner!

@roomrys roomrys merged commit 5c3441c into develop Oct 19, 2023
9 checks passed
@roomrys roomrys deleted the liezl/refactor-add-instance branch October 19, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants