-
Notifications
You must be signed in to change notification settings - Fork 81
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
fix new viewer after image rotation #2677
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2677 +/- ##
=======================================
Coverage 90.85% 90.86%
=======================================
Files 162 162
Lines 21126 21127 +1
=======================================
+ Hits 19195 19196 +1
Misses 1931 1931 ☔ 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.
Looks reasonable. Thanks!
I'll let you decide if you also want Brett to review. If not, feel free to merge. |
|
||
if hasattr(viewer, 'reference'): | ||
viewer.state.reference_data = ref_data | ||
if self.config == 'imviz': |
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.
I did try to generalize this logic instead of hardcoding the config, but not sure its worth it until/unless other configs need this logic? See the commit history to see a failed attempt.
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.
Sensible. Thanks!
* generalize app _on_new_viewer for non-imviz case * limit logic block to only run for imviz
Description
This pull request generalizes the logic for adding a new viewer so other configs/viewers that don't use the concept of reference data and wcs-linking (introduced in #2179) do not raise a traceback.
This is a minor change to a feature not-yet-released, so does not need a changelog entry of its own (although we could append the PR number to the image rotation PR if a link between the PRs in GitHub is not sufficient papertrail).
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.