Add menu item for deleting instances beyond frame limit #1797
Conversation
WalkthroughA new feature has been introduced to the SLEAP GUI application, allowing users to delete predicted instances beyond a specified frame limit. This involves adding a new menu item, creating a command to handle the deletion, and implementing tests to ensure functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GUI
participant Commands
participant DataHandler
User->>GUI: Clicks "Delete Predictions beyond Frame Limit..."
GUI->>Commands: Trigger deleteFrameLimitPredictions()
Commands->>DataHandler: Identify and delete instances beyond frame limit
DataHandler->>Commands: Confirm deletion
Commands->>GUI: Update display
GUI->>User: Show updated predictions
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 (
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (6)
sleap/gui/app.py (1)
Line range hint
261-261: Remove the unused variablefilenameto clean up the code.- filename = event.mimeData().data(mime_format).data().decode()sleap/gui/commands.py (5)
Line range hint
193-193: Undefined reference toMainWindow.The reference to
MainWindowin theexecutemethod ofNewProjectclass is undefined within this file. Ensure thatMainWindowis correctly imported or defined to avoid runtime errors.
Line range hint
817-817: Unused variablefile_dir.The variable
file_dirin theaskmethod ofImportNWBis assigned but never used. Consider removing it if it's not needed.
Line range hint
1691-1691: Avoid using bareexcept.Using a bare
exceptis not recommended as it can catch unexpected exceptions and hide programming errors. Specify the exception types that you expect to handle.Also applies to: 1712-1712
Line range hint
2418-2418: Remove unnecessary f-string.The f-string in this line does not contain any placeholders, making the use of an f-string unnecessary. Consider using a regular string instead.
Also applies to: 2787-2787
Line range hint
3149-3149: Avoid using bareexcept.Using a bare
exceptis not recommended as it can catch unexpected exceptions and hide programming errors. Specify the exception types that you expect to handle.
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
filenameis assigned to but never used (F841)sleap/gui/commands.py
193-193: Undefined name
MainWindow(F821)
817-817: Local variable
file_diris assigned to but never used (F841)
1691-1691: Do not use bare
except(E722)
1712-1712: Do not use bare
except(E722)
2418-2418: f-string without any placeholders (F541)
2787-2787: f-string without any placeholders (F541)
3149-3149: Do not use bare
except(E722)
Additional comments not posted (1)
sleap/gui/app.py (1)
793-795: The addition of the new menu item "Delete Predictions beyond Frame Limit new item..." is correctly implemented and linked to the appropriate command.
| def deleteFrameLimitPredictions(self): | ||
| """Gui for deleting instances beyond some frame number.""" | ||
| self.execute(DeleteFrameLimitPredictions) | ||
|
|
There was a problem hiding this comment.
Add method documentation for deleteFrameLimitPredictions.
Consider adding a detailed docstring for the deleteFrameLimitPredictions method to explain its parameters, return type, and any exceptions it might raise. This will enhance code maintainability and readability.
sleap/gui/commands.py
Outdated
| class DeleteFrameLimitPredictions(InstanceDeleteCommand): | ||
| @staticmethod | ||
| def get_frame_instance_list(context: CommandContext, params: Dict): | ||
| predicted_instances = [] | ||
| # Select the instances to be deleted | ||
| for lf in context.labels.find(context.state["video"]): | ||
| if lf.frame_idx >= params["frame_idx_threshold"]: | ||
| predicted_instances.extend( | ||
| [(lf, inst) for inst in lf.predicted_instances] | ||
| ) | ||
| return predicted_instances | ||
|
|
||
| @classmethod | ||
| def ask(cls, context: CommandContext, params: Dict) -> bool: | ||
| current_video = context.state["video"] | ||
| frame_idx_thresh, okay = QtWidgets.QInputDialog.getInt( | ||
| context.app, | ||
| "Delete Instance beyond Frame Number...", | ||
| "Frame number after which instances to be deleted:", | ||
| 1, | ||
| 1, | ||
| len(current_video), | ||
| ) | ||
| if okay: | ||
| params["frame_idx_threshold"] = frame_idx_thresh | ||
| return super().ask(context, params) | ||
|
|
There was a problem hiding this comment.
Ensure proper validation and error handling in DeleteFrameLimitPredictions.
- if okay:
+ if okay and frame_idx_thresh > 0:It's crucial to validate the frame_idx_threshold to ensure it's a positive integer. This prevents logical errors where a negative frame index could potentially delete all instances.
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.
| class DeleteFrameLimitPredictions(InstanceDeleteCommand): | |
| @staticmethod | |
| def get_frame_instance_list(context: CommandContext, params: Dict): | |
| predicted_instances = [] | |
| # Select the instances to be deleted | |
| for lf in context.labels.find(context.state["video"]): | |
| if lf.frame_idx >= params["frame_idx_threshold"]: | |
| predicted_instances.extend( | |
| [(lf, inst) for inst in lf.predicted_instances] | |
| ) | |
| return predicted_instances | |
| @classmethod | |
| def ask(cls, context: CommandContext, params: Dict) -> bool: | |
| current_video = context.state["video"] | |
| frame_idx_thresh, okay = QtWidgets.QInputDialog.getInt( | |
| context.app, | |
| "Delete Instance beyond Frame Number...", | |
| "Frame number after which instances to be deleted:", | |
| 1, | |
| 1, | |
| len(current_video), | |
| ) | |
| if okay: | |
| params["frame_idx_threshold"] = frame_idx_thresh | |
| return super().ask(context, params) | |
| class DeleteFrameLimitPredictions(InstanceDeleteCommand): | |
| @staticmethod | |
| def get_frame_instance_list(context: CommandContext, params: Dict): | |
| predicted_instances = [] | |
| # Select the instances to be deleted | |
| for lf in context.labels.find(context.state["video"]): | |
| if lf.frame_idx >= params["frame_idx_threshold"]: | |
| predicted_instances.extend( | |
| [(lf, inst) for inst in lf.predicted_instances] | |
| ) | |
| return predicted_instances | |
| @classmethod | |
| def ask(cls, context: CommandContext, params: Dict) -> bool: | |
| current_video = context.state["video"] | |
| frame_idx_thresh, okay = QtWidgets.QInputDialog.getInt( | |
| context.app, | |
| "Delete Instance beyond Frame Number...", | |
| "Frame number after which instances to be deleted:", | |
| 1, | |
| 1, | |
| len(current_video), | |
| ) | |
| if okay and frame_idx_thresh > 0: | |
| params["frame_idx_threshold"] = frame_idx_thresh | |
| return super().ask(context, params) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1797 +/- ##
===========================================
+ Coverage 73.30% 74.16% +0.85%
===========================================
Files 134 135 +1
Lines 24087 24575 +488
===========================================
+ Hits 17658 18226 +568
+ Misses 6429 6349 -80 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (22)
tests/gui/test_commands.py (6)
Line range hint
76-76: Remove duplicate items from sets to ensure data integrity.- "tests/data/dlc_multiple_datasets/video1/img000.jpg", - "tests/data/dlc_multiple_datasets/video1/img000.jpg", + "tests/data/dlc_multiple_datasets/video1/img000.jpg",- {0, 0, 1} + {0, 1}Also applies to: 79-79
Line range hint
134-137: Simplify the assignment ofcsvusing a ternary operator.- if out_suffix == "csv": - csv = True - else: - csv = False + csv = True if out_suffix == "csv" else False
Line range hint
225-225: Replace equality comparisons toTruewith direct truth checks.- assert okay == True + assert okayAlso applies to: 234-234, 242-242, 251-251, 263-263, 272-272, 279-279, 297-297
Line range hint
358-358: Remove the unused variablelast_lf_frameto clean up the code.- last_lf_frame = get_last_lf_in_video(labels, videos[0])
Line range hint
366-366: Use direct truth checks instead of comparing toTrue.- assert video.backend.grayscale == True + assert video.backend.grayscale
Line range hint
526-526: Remove the extraneousfprefix from the string since there are no placeholders.- default_name += f".{adaptor.default_ext}" + default_name += ".{adaptor.default_ext}"sleap/gui/app.py (6)
Line range hint
123-123: Usesuper()instead ofsuper(__class__, self).- super(MainWindow, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs)
Line range hint
194-194: Usesuper()instead ofsuper(__class__, self).- super(MainWindow, self).setWindowTitle(f"{value} - SLEAP v{sleap.version.__version__}") + super().setWindowTitle(f"{value} - SLEAP v{sleap.version.__version__}")
Line range hint
209-210: Combine nestedifstatements usingand.- if e.type() == QEvent.StatusTip: - if e.tip() == "": + if e.type() == QEvent.StatusTip and e.tip() == "":
Line range hint
261-261: Local variablefilenameis assigned to but never used.- filename = event.mimeData().data(mime_format).data().decode()
Line range hint
1087-1087: Remove extraneous parentheses.- if (n_instances > 0) and not self.state["show instances"]: + if n_instances > 0 and not self.state["show instances"]:
Line range hint
1148-1151: Use a more concise expression withany().- def _has_topic(topic_list): - if UpdateTopic.all in what: - return True - for topic in topic_list: - if topic in what: - return True - return False + def _has_topic(topic_list): + return UpdateTopic.all in what or any(topic in what for topic in topic_list)sleap/gui/commands.py (10)
Line range hint
193-193: Undefined reference toMainWindow.Please ensure that
MainWindowis defined or imported in this file, as it is referenced in theCommandContextclass.
Line range hint
653-653: Simplify dictionary access.- params.get("filename", None) + params.get("filename")
Line range hint
756-756: Simplify dictionary access.- video_path = params["video_path"] if "video_path" in params else None + video_path = params.get("video_path", None)
Line range hint
817-817: Remove unused variable.- file_dir = os.path.dirname(filename)The variable
file_diris assigned but never used. Consider removing it to clean up the code.
Line range hint
1691-1691: Avoid using bareexcept.Using bare
exceptcan catch unexpected exceptions and hide programming errors. Specify the exception type or useexcept Exceptionto catch all standard errors.Also applies to: 1712-1712
Line range hint
1972-1972: Improve exception chaining.- except Exception as e: + except Exception as e: + raise RuntimeError("Failed to save project.") from eUse
raise ... fromto provide context for the exception and help with debugging.
Line range hint
2101-2101: Simplify dictionary key check.- if node not in instance.nodes.keys(): + if node not in instance.nodes:You can directly check for the existence of a key in a dictionary without using
.keys().Also applies to: 2116-2116
Line range hint
2418-2418: Remove unnecessary f-string prefix.- f"This video type {type(video.backend)} for video at index {idx} " + "This video type {type(video.backend)} for video at index {idx} "The f-string is used without placeholders, so the prefix can be removed.
Also applies to: 2789-2789
Line range hint
2887-2887: Simplify dictionary access.- params.get("copy_instance", None) + params.get("copy_instance") - params.get("location", None) + params.get("location")Also applies to: 2889-2889
Line range hint
3151-3151: Avoid using bareexcept.Using bare
exceptcan catch unexpected exceptions and hide programming errors. Specify the exception type or useexcept Exceptionto catch all standard errors.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- sleap/gui/app.py (1 hunks)
- sleap/gui/commands.py (2 hunks)
- tests/gui/test_commands.py (2 hunks)
Additional context used
Ruff
tests/gui/test_commands.py
72-72: Ambiguous variable name:
l(E741)
76-76: Sets should not contain duplicate item
"tests/data/dlc_multiple_datasets/video1/img000.jpg"(B033)Remove duplicate item
79-79: Ambiguous variable name:
l(E741)
79-79: Sets should not contain duplicate item
0(B033)Remove duplicate item
134-137: Use ternary operator
csv = True if out_suffix == "csv" else Falseinstead ofif-else-block (SIM108)Replace
if-else-block withcsv = True if out_suffix == "csv" else False
200-200: Loop control variable
videonot used within loop body (B007)Rename unused
videoto_video
225-225: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
234-234: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
242-242: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
251-251: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
263-263: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
272-272: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
279-279: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
297-297: Avoid equality comparisons to
True; useif okay:for truth checks (E712)Replace with
okay
303-303: Loop control variable
videonot used within loop body (B007)Rename unused
videoto_video
358-358: Local variable
last_lf_frameis assigned to but never used (F841)Remove assignment to unused variable
last_lf_frame
366-366: Avoid equality comparisons to
True; useif video.backend.grayscale:for truth checks (E712)Replace with
video.backend.grayscale
526-526: f-string without any placeholders (F541)
Remove extraneous
fprefixsleap/gui/app.py
123-123: Use
super()instead ofsuper(__class__, self)(UP008)Remove
__super__parameters
194-194: Use
super()instead ofsuper(__class__, self)(UP008)Remove
__super__parameters
209-210: Use a single
ifstatement instead of nestedifstatements (SIM102)Combine
ifstatements usingand
261-261: Local variable
filenameis assigned to but never used (F841)Remove assignment to unused variable
filename
1087-1087: Avoid extraneous parentheses (UP034)
Remove extraneous parentheses
1148-1151: Use
return any(topic in what for topic in topic_list)instead offorloop (SIM110)Replace with
return any(topic in what for topic in topic_list)sleap/gui/commands.py
193-193: Undefined name
MainWindow(F821)
653-653: Use
params.get("filename")instead ofparams.get("filename", None)(SIM910)Replace
params.get("filename", None)withparams.get("filename")
756-756: Use
params.get("video_path", None)instead of anifblock (SIM401)Replace with
params.get("video_path", None)
817-817: Local variable
file_diris assigned to but never used (F841)Remove assignment to unused variable
file_dir
1691-1691: Do not use bare
except(E722)
1712-1712: Do not use bare
except(E722)
1972-1972: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
2101-2101: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2116-2116: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2418-2418: f-string without any placeholders (F541)
Remove extraneous
fprefix
2789-2789: f-string without any placeholders (F541)
Remove extraneous
fprefix
2887-2887: Use
params.get("copy_instance")instead ofparams.get("copy_instance", None)(SIM910)Replace
params.get("copy_instance", None)withparams.get("copy_instance")
2889-2889: Use
params.get("location")instead ofparams.get("location", None)(SIM910)Replace
params.get("location", None)withparams.get("location")
3151-3151: Do not use bare
except(E722)
Additional comments not posted (3)
tests/gui/test_commands.py (2)
19-19: The import ofDeleteFrameLimitPredictionsis correctly placed in the import section. Ensure that all necessary methods from this class are covered by unit tests.
851-871: The new test functiontest_DeleteFrameLimitPredictionsis well-structured and tests the functionality as described in the PR. Ensure that theexpected_instancesvalue is derived from a reliable source or calculation to maintain test accuracy.sleap/gui/app.py (1)
791-796: Ensure the new menu item "Delete Predictions beyond Frame Limit..." is correctly integrated and accessible.This addition aligns with the PR's objective to enhance usability by allowing users to delete predictions beyond a specified frame limit. Please ensure that this new menu item is tested thoroughly to confirm its functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (10)
sleap/gui/commands.py (10)
Line range hint
193-193: Undefined reference toMainWindow.The reference to
MainWindowin the methodexecuteof theAppCommandclass is undefined within this file. Ensure thatMainWindowis properly imported or defined, or use a different existing reference.- MainWindow + [Correct reference or import statement]
Line range hint
653-653: Simplify dictionary access.Use
params.get("filename")instead ofparams.get("filename", None)for simplicity and readability.- params.get("filename", None) + params.get("filename")
Line range hint
756-756: Simplify dictionary access.Use
params.get("video_path", None)instead of anifblock for simplicity and readability.- video_path = params["video_path"] if "video_path" in params else None + video_path = params.get("video_path", None)
Line range hint
817-817: Remove unused variable.The local variable
file_diris assigned but never used. Consider removing it to clean up the code.- file_dir = os.path.dirname(filename)
Line range hint
1691-1691: Avoid using bareexcept.Using a bare
exceptcan catch unexpected exceptions and hide programming errors. Specify the exception type to improve error handling.- except: + except Exception as e:Also applies to: 1712-1712
Line range hint
1972-1972: Improve exception chaining.Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling.- raise TypeError("Importing videos with different extensions is not supported.") + raise TypeError("Importing videos with different extensions is not supported.") from None
Line range hint
2101-2101: Usekey in dictinstead ofkey in dict.keys().For checking if a key is in a dictionary, it's more idiomatic and slightly faster to use
key in dictinstead ofkey in dict.keys().- if node not in instance.nodes.keys(): + if node not in instance.nodes:Also applies to: 2116-2116
Line range hint
2418-2418: Remove extraneousfprefix from strings.The
fprefix is used for formatted strings in Python, but it's unnecessary when the string does not contain any placeholders.- 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: 2789-2789
Line range hint
2887-2887: Simplify dictionary access.Use
params.get("copy_instance")andparams.get("location")instead ofparams.get("copy_instance", None)andparams.get("location", None)for simplicity and readability.- params.get("copy_instance", None) + params.get("copy_instance") - params.get("location", None) + params.get("location")Also applies to: 2889-2889
Line range hint
3151-3151: Avoid using bareexcept.Using a bare
exceptcan catch unexpected exceptions and hide programming errors. Specify the exception type to improve error handling.- except: + except Exception as e:
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/gui/commands.py (2 hunks)
Additional context used
Ruff
sleap/gui/commands.py
193-193: Undefined name
MainWindow(F821)
653-653: Use
params.get("filename")instead ofparams.get("filename", None)(SIM910)Replace
params.get("filename", None)withparams.get("filename")
756-756: Use
params.get("video_path", None)instead of anifblock (SIM401)Replace with
params.get("video_path", None)
817-817: Local variable
file_diris assigned to but never used (F841)Remove assignment to unused variable
file_dir
1691-1691: Do not use bare
except(E722)
1712-1712: Do not use bare
except(E722)
1972-1972: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
2101-2101: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2116-2116: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2418-2418: f-string without any placeholders (F541)
Remove extraneous
fprefix
2789-2789: f-string without any placeholders (F541)
Remove extraneous
fprefix
2887-2887: Use
params.get("copy_instance")instead ofparams.get("copy_instance", None)(SIM910)Replace
params.get("copy_instance", None)withparams.get("copy_instance")
2889-2889: Use
params.get("location")instead ofparams.get("location", None)(SIM910)Replace
params.get("location", None)withparams.get("location")
3151-3151: Do not use bare
except(E722)
Additional comments not posted (2)
sleap/gui/commands.py (2)
497-500: Add method documentation fordeleteFrameLimitPredictions.Consider adding a detailed docstring for the
deleteFrameLimitPredictionsmethod to explain its parameters, return type, and any exceptions it might raise. This will enhance code maintainability and readability.
2476-2487: Add a comment that this is used in theInstanceDeleteCommand.askmethod.
sleap/gui/commands.py
Outdated
| """Called from the parent `InstanceDeleteCommand.ask` class. Returns a list of | ||
| instances to be deleted.""" |
There was a problem hiding this comment.
Have a single line summary docstring followed by Args and Returns if needed.
…ps://github.com/talmolab/sleap into shrivaths/delete-predictions-beyond-frame-limit
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (10)
sleap/gui/commands.py (10)
Line range hint
193-193: Undefined reference toMainWindow.- """Entry point for running command. + """Entry point for running command in `MainWindow`.Ensure that
MainWindowis correctly imported or defined within the scope where it's referenced.
Line range hint
653-653: Simplify thegetmethod for dictionary access.- filename = params.get("filename", None) # If called with just a Labels object + filename = params.get("filename") # If called with just a Labels object
Line range hint
756-756: Use thegetmethod with a default value directly.- video_path = params["video_path"] if "video_path" in params else None + video_path = params.get("video_path", None)
Line range hint
817-817: Remove assignment to unused variablefile_dir.- file_dir = os.path.dirname(filename)
Line range hint
1691-1691: Avoid using bareexceptstatements. Specify exception types to handle specific exceptions appropriately.- except: + except Exception as e:Also applies to: 1712-1712
Line range hint
1972-1972: When re-raising exceptions, useraise ... from errto provide context.- raise e + raise Exception("Error during save operation.") from e
Line range hint
2101-2101: Usekey in dictinstead ofkey in dict.keys().- if node not in instance.nodes.keys(): + if node not in instance.nodes:Also applies to: 2116-2116
Line range hint
2418-2418: Remove extraneousfprefix from strings that do not contain placeholders.- f"This video type {type(video.backend)} for video at index {idx} " + "This video type {type(video.backend)} for video at index {idx} "Also applies to: 2792-2792
Line range hint
2890-2890: Simplify thegetmethod for dictionary access.- copy_instance = params.get("copy_instance", None) + copy_instance = params.get("copy_instance") - location = params.get("location", None) + location = params.get("location")Also applies to: 2892-2892
Line range hint
3154-3154: Avoid using bareexceptstatements. Specify exception types to handle specific exceptions appropriately.- except: + except Exception as e:
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/gui/commands.py (2 hunks)
Additional context used
Ruff
sleap/gui/commands.py
193-193: Undefined name
MainWindow(F821)
653-653: Use
params.get("filename")instead ofparams.get("filename", None)(SIM910)Replace
params.get("filename", None)withparams.get("filename")
756-756: Use
params.get("video_path", None)instead of anifblock (SIM401)Replace with
params.get("video_path", None)
817-817: Local variable
file_diris assigned to but never used (F841)Remove assignment to unused variable
file_dir
1691-1691: Do not use bare
except(E722)
1712-1712: Do not use bare
except(E722)
1972-1972: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
2101-2101: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2116-2116: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2418-2418: f-string without any placeholders (F541)
Remove extraneous
fprefix
2792-2792: f-string without any placeholders (F541)
Remove extraneous
fprefix
2890-2890: Use
params.get("copy_instance")instead ofparams.get("copy_instance", None)(SIM910)Replace
params.get("copy_instance", None)withparams.get("copy_instance")
2892-2892: Use
params.get("location")instead ofparams.get("location", None)(SIM910)Replace
params.get("location", None)withparams.get("location")
3154-3154: Do not use bare
except(E722)
Additional comments not posted (2)
sleap/gui/commands.py (2)
497-500: Consider adding a detailed docstring for thedeleteFrameLimitPredictionsmethod to explain its parameters, return type, and any exceptions it might raise. This will enhance code maintainability and readability.
2476-2477: Add a comment that this is used in theInstanceDeleteCommand.askmethod.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (10)
sleap/gui/commands.py (10)
Line range hint
193-193: Specify correct type hint forMainWindow.The type hint for
MainWindowis missing, causing an undefined name issue. Please importMainWindowor define it if it's a custom class.+ from sleap.gui.mainwindow import MainWindow
Line range hint
653-653: Simplify dictionary access.Directly use
params.get("filename")instead ofparams.get("filename", None)asNoneis the default return value for thegetmethod.- filename = params.get("filename", None) # If called with just a Labels object + filename = params.get("filename") # If called with just a Labels object
Line range hint
756-756: Simplify dictionary access.Replace the
ifblock withparams.get("video_path", None)for cleaner and more efficient code.- video_path = params["video_path"] if "video_path" in params else None + video_path = params.get("video_path", None)
Line range hint
817-817: Remove unused variable.The variable
file_diris assigned but never used. Consider removing it to clean up the code.- file_dir = os.path.dirname(filename)
Line range hint
1691-1691: Avoid using bareexceptstatements.Using bare
exceptstatements can catch unexpected exceptions and hide programming errors. Specify the exception type or useexcept Exceptionto catch all standard exceptions.- except: + except Exception:Also applies to: 1712-1712
Line range hint
1972-1972: Improve exception chaining for clarity.When re-raising exceptions, use the
fromkeyword to clarify whether the new exception was directly caused by the previous one.- except Exception as e: + except Exception as e: + raise RuntimeError("Failed to save") from e
Line range hint
2101-2101: Simplify dictionary key check.Use
key in dictinstead ofkey in dict.keys()for more concise and pythonic code.- if node in instance.nodes.keys(): + if node in instance.nodes:Also applies to: 2116-2116
Line range hint
2420-2420: Remove unnecessary f-string prefix.The f-string syntax is used without placeholders, which is unnecessary. Remove the
fprefix.- f"This video type {type(video.backend)} for video at index {idx} " + "This video type {type(video.backend)} for video at index {idx} "Also applies to: 2796-2796
Line range hint
2894-2894: Simplify dictionary access.Use
params.get("key")directly as it is cleaner and the default return isNone.- copy_instance = params.get("copy_instance", None) + copy_instance = params.get("copy_instance") - location = params.get("location", None) + location = params.get("location")Also applies to: 2896-2896
Line range hint
3158-3158: Avoid using bareexceptstatements.Replace bare
exceptwithexcept Exceptionto avoid catching unexpected exceptions.- except: + except Exception:
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/gui/commands.py (2 hunks)
Additional context used
Ruff
sleap/gui/commands.py
193-193: Undefined name
MainWindow(F821)
653-653: Use
params.get("filename")instead ofparams.get("filename", None)(SIM910)Replace
params.get("filename", None)withparams.get("filename")
756-756: Use
params.get("video_path", None)instead of anifblock (SIM401)Replace with
params.get("video_path", None)
817-817: Local variable
file_diris assigned to but never used (F841)Remove assignment to unused variable
file_dir
1691-1691: Do not use bare
except(E722)
1712-1712: Do not use bare
except(E722)
1972-1972: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
2101-2101: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2116-2116: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
2420-2420: f-string without any placeholders (F541)
Remove extraneous
fprefix
2796-2796: f-string without any placeholders (F541)
Remove extraneous
fprefix
2894-2894: Use
params.get("copy_instance")instead ofparams.get("copy_instance", None)(SIM910)Replace
params.get("copy_instance", None)withparams.get("copy_instance")
2896-2896: Use
params.get("location")instead ofparams.get("location", None)(SIM910)Replace
params.get("location", None)withparams.get("location")
3158-3158: Do not use bare
except(E722)
Additional comments not posted (2)
sleap/gui/commands.py (2)
497-500: Add method documentation fordeleteFrameLimitPredictions.Consider adding a detailed docstring for the
deleteFrameLimitPredictionsmethod to explain its parameters, return type, and any exceptions it might raise. This will enhance code maintainability and readability.
2478-2479: Add a comment that this is used in theInstanceDeleteCommand.askmethod.This comment helps clarify the relationship between the
DeleteFrameLimitPredictionsclass and its superclass, making the code easier to understand.
Description
Currently there is no menu option to delete predictions beyond a certain frame number. This would add some ease of use when trying to delete predictions and then re-labeling to train the model again to get different/better predictions.
This came in as a suggestion in #1762
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
New Features
Tests