-
Notifications
You must be signed in to change notification settings - Fork 88
Add viewer.get_viewport_region method #3851
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3851 +/- ##
==========================================
+ Coverage 87.63% 87.64% +0.01%
==========================================
Files 188 188
Lines 25703 25725 +22
==========================================
+ Hits 22524 22547 +23
+ Misses 3179 3178 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Kyle Conroy <[email protected]>
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.
A few thoughts...
- Would it ever be useful to get pixel coordinates of a specific layer with this as well (obviously pixel coordinates of the reference layer are the same as the axes limits)?
- This is slightly different, but not all that different, from
get_viewport. This returns the corners instead of center and radius, but doesn't have the flexibility mentioned above to request a specific layer or toggle between sky and pixel coordinates. Do we want to somehow merge these or include corner information inget_viewport(modulo my next concern) - This will be susceptible to multiple widget instances of the same image viewer and I think will just return the corners of the most recently focused viewer (but we probably should confirm that). Is that ok? Do we need to explain that anywhere or is in self-explanatory?
|
Good Qs @kecnry: I'm sure someone could find a use for pixel regions as well, which is just the first half of This will be susceptible to multiple widget instances. The Cobalt workflow that uses this method, showing a jdaviz viewer's footprint in another app, will always occur in a crowded jupyterlab workspace, where it would be hard (though not impossible!) to have multiple views of a widget. |
ab79392 to
931b63a
Compare
931b63a to
6616a59
Compare
Description
Introducing: a method on the imviz image viewer
get_viewport_regionwhich returns aPolygonSkyRegionthat outlines the imviz viewer. This PR is the jdaviz-side implementation of spacetelescope/mast-aladin#71. Together, we can use these to show one app's viewport limits in another app (specifically: jdaviz <-> mast-aladin).Test it locally with this in one cell:
Then run
Change log entry
CHANGES.rst? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rstbefore merge. If no, maintainershould add a
no-changelog-entry-neededlabel.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.
triviallabel.cache-download.ymlworkflow?