-
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
Add highlight for instance group dock #1860
Add highlight for instance group dock #1860
Conversation
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? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## liezl/add-gui-elements-for-sessions #1860 +/- ##
=======================================================================
- Coverage 74.18% 74.18% -0.01%
=======================================================================
Files 135 135
Lines 25425 25438 +13
=======================================================================
+ Hits 18862 18871 +9
- Misses 6563 6567 +4 ☔ 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.
If we make these changes, then things should be in a working state. Also, make sure to run black
on sleap/gui/widgets/video.py
(and any other file you change - if any)
sleap/gui/widgets/video.py
Outdated
elif isinstance(select, Instance): | ||
instance.selected = select == instance.instance | ||
elif isinstance(select, InstanceGroup): | ||
instance.selected = True if instance in select else False | ||
instance.selected = select == idx or select == instance.instance |
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.
Remove the old logic otherwise you are just overwriting your changes from above:
instance.selected = select == idx or select == instance.instance |
sleap/gui/widgets/video.py
Outdated
elif isinstance(select, Instance): | ||
instance.selected = select == instance.instance | ||
elif isinstance(select, InstanceGroup): | ||
instance.selected = True if instance in select else 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.
InstanceGroup
stores Instance
objects not QtInstance
objects, so we need to use the QtInstance.instance
attribute to get the associated Instance
.
Also, when using the in
keyword in Python, the dunder method __contains__
is called on the object that is being checked for containment. If __contains__
is not implemented, Python falls back to using the __iter__
method to iterate over the elements (if available) and checks each element. If __iter__
is also not implemented, Python then tries to use the __getitem__
method to check for containment by iterating over index values starting from zero.
Neither InstanceGroup.__contains__
nor InstanceGroup.__iter__
is implemented and InstanceGroup.__getitem__
unfortunately only handles this in the case of the Instance
selected being the first instance in InstanceGroup.instances
(we iterate to too large a number otherwise and get a KeyError
). Let's just use in InstanceGroup.instances
for now.
instance.selected = True if instance in select else False | |
instance.selected = True if instance.instance in select.instances else 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.
Bug has been located. Sorry if my comments mislead you!
sleap/gui/widgets/video.py
Outdated
instance.selected = select == idx | ||
elif isinstance(select, Instance): | ||
instance.selected = select == instance.instance | ||
elif isinstance(select, QtInstance.instance): |
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 cannot access an instance attribute on a class. If you have some time, I would suggest reading this article on instance attributes.
elif isinstance(select, QtInstance.instance): | |
elif isinstance(select, QtInstance): |
…com/talmolab/sleap into justin/add-highlight-for-InstanceGroupDock
69e00b1
into
liezl/add-gui-elements-for-sessions
Description
add highlighting feature for row in instance group dock
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️