jdaviz
jdaviz copied to clipboard
Astrowidgets API: Markers to use Astropy regions
Description
This pull request is to explore what if markers use Astropy regions instead of Astropy Table. There is no expectation for this to be merged until more backends are explored and then API is finalized upstream in https://github.com/astropy/astrowidgets
Related issues:
- https://github.com/astropy/astrowidgets/issues/137
- https://github.com/astropy/astrowidgets/issues/51
Pros:
- Markers no longer cause data tables to be added to app data collection. They are now pure bqplot figure marks. This probably also improved performance.
- Linking can now change link type with or without markers. This also means we can now get rid of
MarkersChangedMessage
. - When you zoom in now, marker gets larger. That is, marker now follows data size, not viewer.
Cons:
- You can add sky regions but it does not respect WCS linking because under the hood, everything gets turned into pixels for bqplot.
- We need to render all the shapes into polygons for bqplot so the shape follows data size, not viewer. This means we need to track the input regions separately to preserve API roundtrip.
- API changes are totally not backward compatible.
- Afraid to expose more customization (e.g., alpha for the fill color) in fear of causing Astrowidgets API to be unrealistic for other backends out there.
- Can be easily confused with Footprints and Markers plugins. (This is already a problem regardless of this change or not but might as well list it.)
Change log entry
- [x] Is a change log needed? If yes, is it added to
CHANGES.rst
? If you want to avoid merge conflicts, list the proposed change log here for review and add toCHANGES.rst
before merge. If no, maintainer should add ano-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.
- [x] Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the
trivial
label. - [ ] Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
- [x] Do the proposed changes follow the STScI Style Guides?
- [x] Are tests added/updated as required? If so, do they follow the STScI Style Guides?
- [x] Are docs added/updated as required? If so, do they follow the STScI Style Guides?
- [ ] Did the CI pass? If not, are the failures related?
- [x] Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
- [ ] After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?