-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add color by instance group preference saving #1999
Add color by instance group preference saving #1999
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## liezl/add-gui-elements-for-sessions #1999 +/- ##
=======================================================================
- Coverage 74.18% 74.18% -0.01%
=======================================================================
Files 135 135
Lines 25438 25440 +2
=======================================================================
+ Hits 18872 18873 +1
- Misses 6566 6567 +1 ☔ View full report in Codecov by Sentry. |
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.
Almost. Let's rename "color_by_instance_group"
to "distinctly_color"
which already exists as a GuiState
key and is connected to the existing View > Apply Distinct Colors to submenu. Also, we don't want to save the preferences file each time we change the checkbox selection.
sleap/gui/app.py
Outdated
|
||
def _save_color_by_instance_group(self, value): | ||
"""Update the color_by_instance_group preference and save it.""" | ||
prefs["color_by_instance_group"] = value | ||
prefs.save() |
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.
We do not want to save preferences each time we click a checkbox, and we already have logic above to save on close.
Lines 215 to 229 in b55d4ef
def closeEvent(self, event): | |
"""Close application window, prompting for saving as needed.""" | |
# Save window state. | |
prefs["window state"] = self.saveState() | |
prefs["marker size"] = self.state["marker size"] | |
prefs["show non-visible nodes"] = self.state["show non-visible nodes"] | |
prefs["node label size"] = self.state["node label size"] | |
prefs["edge style"] = self.state["edge style"] | |
prefs["propagate track labels"] = self.state["propagate track labels"] | |
prefs["color predicted"] = self.state["color predicted"] | |
prefs["trail shade"] = self.state["trail_shade"] | |
prefs["share usage data"] = self.state["share usage data"] | |
# Save preferences. | |
prefs.save() |
def _save_color_by_instance_group(self, value): | |
"""Update the color_by_instance_group preference and save it.""" | |
prefs["color_by_instance_group"] = value | |
prefs.save() |
sleap/gui/app.py
Outdated
add_menu_check_item(viewMenu, "color_by_instance_group", "Color by Instance Group") | ||
self.state.connect("color_by_instance_group", lambda value: self._save_color_by_instance_group(value)) | ||
|
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.
We already have a GUI element for this. It's under View > Apply DIstinct COlors to > instance_groups and connected to state["distinctly_color"]
:
Lines 658 to 668 in b55d4ef
distinctly_color_options = ("instance_groups", "instances", "nodes", "edges") | |
add_submenu_choices( | |
menu=viewMenu, | |
title="Apply Distinct Colors To", | |
options=distinctly_color_options, | |
key="distinctly_color", | |
) | |
self.state["palette"] = prefs["palette"] | |
self.state["distinctly_color"] = "instances" |
add_menu_check_item(viewMenu, "color_by_instance_group", "Color by Instance Group") | |
self.state.connect("color_by_instance_group", lambda value: self._save_color_by_instance_group(value)) |
sleap/gui/app.py
Outdated
@@ -298,6 +304,8 @@ def labels(self, value): | |||
def _initialize_gui(self): | |||
"""Creates menus, dock windows, starts timers to update gui state.""" | |||
|
|||
self.state["color_by_instance_group"] = prefs["color_by_instance_group"] |
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.
The GUI element View > Apply DIstinct COlors to is connected to the state["distinctly_color"]
:
Lines 658 to 668 in b55d4ef
distinctly_color_options = ("instance_groups", "instances", "nodes", "edges") | |
add_submenu_choices( | |
menu=viewMenu, | |
title="Apply Distinct Colors To", | |
options=distinctly_color_options, | |
key="distinctly_color", | |
) | |
self.state["palette"] = prefs["palette"] | |
self.state["distinctly_color"] = "instances" |
self.state["color_by_instance_group"] = prefs["color_by_instance_group"] |
sleap/prefs.py
Outdated
@@ -15,6 +15,7 @@ class Preferences(object): | |||
"medium step size": 10, | |||
"large step size": 100, | |||
"color predicted": False, | |||
"color_by_instance_group": False, |
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.
We do not want a boolean here. We want one of the four str
options for which to distinctly color instances by:
Line 658 in b55d4ef
distinctly_color_options = ("instance_groups", "instances", "nodes", "edges") |
"color_by_instance_group": False, | |
"distinctly_color": "instances", |
sleap/gui/app.py
Outdated
@@ -224,6 +224,7 @@ def closeEvent(self, event): | |||
prefs["color predicted"] = self.state["color predicted"] | |||
prefs["trail shade"] = self.state["trail_shade"] | |||
prefs["share usage data"] = self.state["share usage data"] | |||
prefs["color_by_instance_group"] = self.state["color_by_instance_group"] |
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.
prefs["color_by_instance_group"] = self.state["color_by_instance_group"] | |
prefs["distinctly_color"] = self.state["distinctly_color"] |
…the existing View > Apply Distinct Colors To submenu
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@@ -665,7 +668,7 @@ def prev_vid(): | |||
) | |||
|
|||
self.state["palette"] = prefs["palette"] | |||
self.state["distinctly_color"] = "instances" | |||
self.state["distinctly_color"] = prefs["distinctly_color"] |
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.
Needed this line to use the saved preference.
c7185c7
into
liezl/add-gui-elements-for-sessions
Description
Added color by instance group preference saving
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️