-
Notifications
You must be signed in to change notification settings - Fork 76
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
data-menu: handle viewer rename properly #3383
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3383 +/- ##
==========================================
- Coverage 88.02% 88.00% -0.02%
==========================================
Files 127 127
Lines 19717 19730 +13
==========================================
+ Hits 17356 17364 +8
- Misses 2361 2366 +5 ☔ View full report in Codecov by Sentry. |
if viewer == msg.old_viewer_ref: | ||
self.viewer[i] = msg.new_viewer_ref | ||
elif self.viewer == msg.old_viewer_ref: | ||
self.viewer = msg.new_viewer_ref |
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.
does this need to set viewer id as well like in data_menu?
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.
no, this only tracks the reference so doesn't need to worry about whether the id has changed or not
This comment was marked as resolved.
This comment was marked as resolved.
* data-menu: handle viewer rename properly
Description
This pull request fixes tracebacks caused by the new data-menu when renaming a viewer (currently only used by downstream lcviz - this fixes broken tests in spacetelescope/lcviz#158 when run locally).
Change log entry
CHANGES.rst
? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rst
before merge. If no, maintainershould add a
no-changelog-entry-needed
label.Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
trivial
label.