markers plugin

[kecnry]

Description

This pull request implements a cycler to choose which data to expose in the mouseover coordinates display in the app-level toolbar as well as a "markers" plugin to log this information into a table component.

Plugin Docs
Plugin API Docs

Demo of potential workflow in Imviz (some UI changes since):

This pull request shows an experimental proof-of-concept for how an interactive markers plugin could be implemented 🐱.

Screen.Recording.2022-11-18.at.2.18.21.PM.mov

The new markers plugin is capable of logging all the information exposed there, as well as the ability to toggle the choice of layer or disable the spectrum marker entirely (see #1986):

Screen.Recording.2023-02-14.at.9.49.36.AM.mov

TODO:

• handle new viewers in Imviz correctly
• only cycle between visible data layers (not subsets)
• decide how to handle conflicting units (in mosviz, for example)
• log slice info in image viewer in cubeviz?
• API docs for Table component
• popout support for table component
• store mouseover information?
• fix accessing value of non-active image layer (in both coords info and markers table)
• expand on basic plugin docs
• test coverage

Potential follow-up efforts: during review, consider if these need to be addressed before merging and which warrant follow-up tickets

• switch (API and UI) to enable marker creation when not opened in tray (or should the markers and keyevent always be active? or should .show() also enable? At the least we should be consistent between this and the "l" event in line profile in Imviz)
• ability to export to a data-collection (scatter) entry, with choice on how to handle linking in Imviz. Could then enable plot options for scatters (change marker, markersize, color, etc). Will need testing in spectrum-viewer and various DatasetSelect components in plugins.
• decide whether to support API table import and whether to replace or stand alongside existing markers API (and what docs/notebooks need updating)
• ability to delete individual entries interactively (with another keypress), from the UI, or from API (in the re-used table subcomponent)
• interplay with catalog plugin (needs discussion - should the two plugins be merged? Should they both export to data collection entries?)
• interplay with line profile plugin (needs discussion and related to how integration works with catalog search, possibly require exporting to data collection entry first and then a dropdown in line profile plugin to select and/or loop through points)
• ability to create subsets with user-provided radius/width (and therefore enable workflows with subset centering and aperture photometry or line analysis plugins)
• cycler in app-toolbar to have more consistent styling and a right-click dropdown for when there are a large number of loaded layers
• consider marker matching color of data (especially useful for multiple spectra overlapping, would need to sync with plot options)
• marker styling options in-plugin (marker, markersize, color) - probably would exclude the ability to do per-data-entry colors.
• per-plugin tools infrastructure to allow clicking instead of keypress
• in-plugin instructions for exporting table into notebook (need to detect and only show if not in standalone mode, and also include in aperture photometry, etc).
• consider hiding "*unreliable" columns by default, or if non-GWCS

Closes #1713, closes #1986

Change log entry

• 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 to CHANGES.rst before merge. If no, maintainer
  should 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.

• 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.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• 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?
• 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)?

[pllim]

As I have said before and I will re-iterate here for the record, the complicated part is to account for all the linking that can happen in the same session. Imagine this scenario:

1. Load 2 dithered images in Imviz and link them by WCS.
2. Be default, you will be looking at last loaded image, so when you mark, X and Y are in the frame of the second image. But keep in mind that this is not the same X and Y as the first image. What you are really marking here is the sky coordinates (which aligns between them).
3. So, what will happen now when you blink to the first image and mark another X and Y? Now, this won't be the X and Y that will match the above step.
4. Now, open a second viewer, load yet another image. To make it more fun, let's say it has no WCS. How is marker expected to behave? Should the existing markers automatically show up?

In terms of keeping track of marker positions, we would need to do this in a similar fashion as we handle spatial subsets. Internally, we need to store them consistent wrt reference image used in linking (not to be confused with reference image of the individual viewers... and also this logic might break down when we allow people to remove data from the app entirely but that is a headache for another day). But then when we get the marker info back out, we need to ask user to select data as a reference frame for the correct pixel and sky coordinates; otherwise, they will be ambiguous.

[pllim]

Also, when people see this feature, they might want extra stuff to be added to the table (like maybe sky and value under the cursor).

[kecnry]

This is just a proof-of-concept for when we revisit in the future for more widespread table/marker support (nothing is new - I just thought having a draft PR would be useful so it isn't lost in one of my local branches). That said, this does log the layer that was active when the marker was created, so we should be able to handle any necessary translation on the fly without linking. We'll just need a clear UI for the user to decide whether they want to keep pixel or sky fixed depending on the link type, or we could always only consider markers "valid"/visible for the layer on which they were created until they are exported as a data collection entry in which case that can be handled via linking.

Copying from the ticket, here is a list of potential follow-up extensions I had in mind when I first put this together (I'll also copy this into the description):

• exporting directly to a data-collection entry with choice on how to handle re-linking in imviz,
• ability to import table into plugin via API,
• ability to delete individual entries interactively or from table,
• per-plugin tools infrastructure to allow clicking without keypress to add/delete entries,
• optional vertical only markers for spectra,
• consolidation with catalogs plugin (is searching a catalog essentially just another way to populate this table? Or do we want catalogs to be more similar to line analysis so that multiple catalog searches can be viewed simultaneously?)

[pllim]

This is just a proof-of-concept for when we revisit in the future

Of course, and thanks for posting what you have so it does not get lost! I am just commenting so we remember the conversation happened when we revisit. 😸

[codecov]

Codecov Report

Patch coverage: 95.98% and project coverage change: +0.10 🎉

Comparison is base (223e74d) 92.17% compared to head (b22a19f) 92.28%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1953      +/-   ##
==========================================
+ Coverage   92.17%   92.28%   +0.10%     
==========================================
  Files         140      143       +3     
  Lines       15394    15782     +388     
==========================================
+ Hits        14190    14565     +375     
- Misses       1204     1217      +13     

Impacted Files	Coverage Δ
jdaviz/core/template_mixin.py	92.87% <88.33%> (+0.02%)	⬆️
jdaviz/configs/default/plugins/markers/markers.py	94.93% <94.93%> (ø)
...z/configs/imviz/plugins/coords_info/coords_info.py	96.64% <96.77%> (+3.07%)	⬆️
...configs/cubeviz/plugins/moment_maps/moment_maps.py	94.87% <100.00%> (ø)
jdaviz/configs/default/plugins/__init__.py	100.00% <100.00%> (ø)
...daviz/configs/default/plugins/collapse/collapse.py	100.00% <100.00%> (ø)
...gins/gaussian_smooth/tests/test_gaussian_smooth.py	100.00% <100.00%> (ø)
jdaviz/configs/default/plugins/markers/__init__.py	100.00% <100.00%> (ø)
...fault/plugins/markers/tests/test_markers_plugin.py	100.00% <100.00%> (ø)
jdaviz/configs/default/plugins/viewers.py	97.01% <100.00%> (+0.06%)	⬆️
... and 1 more

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[camipacifici]

Gave it a test drive and it works beautifully!
I am still unsure about having spatial and spectral markers in the same table in cubeviz, but they are easily filterable, so I am ok with it. Maybe with usage, I will come up with some more concrete science cases.
I noticed that in cubeviz, the spatial markers carry the slice number, but not the slice wavelength and I think I remember we decided to include it, but maybe I am wrong.
I have not tested in mosviz yet.

[pllim]

LGTM now. Thanks for your patience!