-
Notifications
You must be signed in to change notification settings - Fork 96
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
Change menu name to match deleting predictions beyond max instance #1790
Change menu name to match deleting predictions beyond max instance #1790
Conversation
WalkthroughThe recent changes in the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant GUI
participant Commands
User->>GUI: Click "delete max instance predictions"
GUI->>Commands: Call deleteInstanceLimitPredictions()
Commands-->>GUI: Execute deletion logic
GUI-->>User: Update UI to reflect deletions
Poem
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 (
|
There was a problem hiding this 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
Outside diff range and nitpick comments (6)
sleap/gui/app.py (2)
Line range hint
261-261
: Remove the unused local variablefilename
to clean up the code.- filename = event.mimeData().data(mime_format).data().decode()
Line range hint
261-261
: Consider enhancing thedragEnterEvent
method to handle multiple file types or provide feedback when an unsupported file type is dragged.def dragEnterEvent(self, event): mime_format = 'application/x-qt-windows-mime;value="FileName"' if mime_format in event.mimeData().formats(): # This only returns the first filename if multiple files are dropped: # Consider handling multiple files or providing user feedback for unsupported types event.acceptProposedAction()sleap/gui/commands.py (4)
Line range hint
193-193
: Undefined nameMainWindow
used. This might be a typo or a missing import. Please verify the correct reference or import the necessary module.
Line range hint
813-813
: The local variablefile_dir
is assigned but never used. Consider removing this variable if it's not needed.- file_dir = os.path.dirname(filename)
Line range hint
1687-1687
: Using bareexcept
statements is generally not recommended as they can catch unexpected exceptions and make debugging more difficult. Consider specifying the exception types that you expect to handle.- except: + except Exception as e:Also applies to: 1708-1708, 3117-3117
Line range hint
2414-2414
: The f-string does not contain any placeholders, making the use of f-string unnecessary. Consider using a regular string instead.- f"This video type {type(video.backend)} for video at index {idx} does not support grayscale yet." + "This video type does not support grayscale yet."Also applies to: 2755-2755
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- sleap/gui/app.py (1 hunks)
- sleap/gui/commands.py (2 hunks)
Additional context used
Ruff
sleap/gui/app.py
261-261: Local variable
filename
is assigned to but never usedsleap/gui/commands.py
193-193: Undefined name
MainWindow
813-813: Local variable
file_dir
is assigned to but never used
1687-1687: Do not use bare
except
1708-1708: Do not use bare
except
2414-2414: f-string without any placeholders
2755-2755: f-string without any placeholders
3117-3117: Do not use bare
except
Additional comments not posted (3)
sleap/gui/app.py (1)
787-789
: The menu item and command name changes are consistent with the PR objectives and improve clarity.sleap/gui/commands.py (2)
2441-2441
: The implementation ofDeleteInstanceLimitPredictions
correctly handles the deletion of instances beyond a specified count threshold. This aligns with the PR's objective to rename and refactor deletion functionality.
493-495
: The methoddeleteInstanceLimitPredictions
has been correctly renamed and the associated command classDeleteInstanceLimitPredictions
is used appropriately. Ensure that all references to this method and class are updated across the project.Verification successful
The renaming of
deleteFrameLimitPredictions
todeleteInstanceLimitPredictions
andDeleteFrameLimitPredictions
toDeleteInstanceLimitPredictions
has been successfully applied throughout the codebase.
- No occurrences of the old function name
deleteFrameLimitPredictions
were found.- No occurrences of the old class name
DeleteFrameLimitPredictions
were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `deleteInstanceLimitPredictions` and `DeleteInstanceLimitPredictions` are updated. # Test: Search for old function and class names. Expect: No occurrences. rg --type python $'deleteFrameLimitPredictions|DeleteFrameLimitPredictions'Length of output: 107
Script:
#!/bin/bash # Description: Verify all references to `deleteInstanceLimitPredictions` and `DeleteInstanceLimitPredictions` are updated. # Test: Search for old function and class names. Expect: No occurrences. rg 'deleteFrameLimitPredictions|DeleteFrameLimitPredictions'Length of output: 62
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1790 +/- ##
===========================================
+ Coverage 73.30% 74.06% +0.76%
===========================================
Files 134 135 +1
Lines 24087 24500 +413
===========================================
+ Hits 17658 18147 +489
+ Misses 6429 6353 -76 ☔ View full report in Codecov by Sentry. |
Description
Currently we have a menu option
Delete Predictions beyond Frame Limit...
but it is misleading as it asks for the max instances beyond which the predictions are deleted. Hence changing the menu name toDelete Predictions beyond Max Instances...
to be clear.Also, changed the related function names to reflect the same.
This could potentially clear the confusion related to #1762
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️