Viewers now have human-readable IDs

[pllim]

• Fix creating a new viewer does not create a valid reference name #372
• Fix JDAT-1491: Add API to return a list of human-readable viewer name #643

Proof of concept

With this patch, such a thing is now possible (though the contour stuff is another issue, see #530).

>>> # After creating tiled viewers
>>> imviz.app.get_viewer_reference_names()
['viewer-1', None]
>>> imviz.app.get_viewer_ids()
['imviz-0', 'imviz-1']
>>> viewer = imviz.app.get_viewer('viewer-1')
>>> viewer2 = imviz.app.get_viewer_by_id('imviz-1')
>>> viewer2.state.layers[0].contour_lim_helper = viewer.state.layers[0].contour_lim_helper
>>> viewer2.state.layers[0].contour_visible = True

New viewer IDs

With this patch, this is how different layouts now behave.

>>> from jdaviz.configs.cubeviz import CubeViz
>>> x = CubeViz()
>>> print('Reference names:', x.app.get_viewer_reference_names())
Reference names: ['flux-viewer', 'uncert-viewer', 'mask-viewer', 'spectrum-viewer']
>>> print('Viewer IDs:', x.app.get_viewer_ids())
Viewer IDs: ['cubeviz-0', 'cubeviz-1', 'cubeviz-2', 'cubeviz-3']

>>> from jdaviz.configs.imviz import Imviz
>>> x = Imviz()
>>> print('Reference names:', x.app.get_viewer_reference_names())
Reference names: ['viewer-1']
>>> print('Viewer IDs:', x.app.get_viewer_ids())
Viewer IDs: ['imviz-0']

>>> from jdaviz.configs.mosviz import MosViz
>>> x = MosViz()
>>> print('Reference names:', x.app.get_viewer_reference_names())
Reference names: ['image-viewer', 'spectrum-2d-viewer', 'spectrum-viewer', 'table-viewer']
>>> print('Viewer IDs:', x.app.get_viewer_ids())
Viewer IDs: ['mosviz-0', 'mosviz-1', 'mosviz-2', 'mosviz-3']

>>> from jdaviz.configs.specviz import SpecViz
>>> x = SpecViz()
>>> print('Reference names:', x.app.get_viewer_reference_names())
Reference names: ['spectrum-viewer']
>>> print('Viewer IDs:', x.app.get_viewer_ids())
Viewer IDs: ['specviz-0']

>>> from jdaviz.configs.specviz2d import Specviz2d
>>> x = Specviz2d()
>>> print('Reference names:', x.app.get_viewer_reference_names())
Reference names: ['spectrum-2d-viewer', 'spectrum-viewer']
>>> print('Viewer IDs:', x.app.get_viewer_ids())
Viewer IDs: ['specviz2d-0', 'specviz2d-1']

>>> from jdaviz.app import Application
>>> x = Application(configuration='default')
>>> print('Reference names:', x.get_viewer_reference_names())
Reference names: []
>>> print('Viewer IDs:', x.get_viewer_ids())
Viewer IDs: []

[codecov]

Codecov Report

Merging #722 (38e827b) into main (409587a) will increase coverage by 0.31%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #722      +/-   ##
==========================================
+ Coverage   62.12%   62.43%   +0.31%     
==========================================
  Files          65       65              
  Lines        4222     4241      +19     
==========================================
+ Hits         2623     2648      +25     
+ Misses       1599     1593       -6     

Impacted Files	Coverage Δ
jdaviz/app.py	80.14% <100.00%> (+0.94%)	⬆️
jdaviz/core/config.py	70.27% <0.00%> (+2.70%)	⬆️
jdaviz/configs/specviz2d/helper.py	20.00% <0.00%> (+6.66%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 409587a...38e827b. Read the comment docs.

[pllim]

p.s. Did I also accidentally fix https://jira.stsci.edu/browse/JDAT-433 here? 🤔 (Ricky said no.)

[rosteen]

This looks good, and doesn't seem to interfere with the way the other viz tools work under the hood. I don't think it closes the ticket you referenced (being able to rename viewers) - I think the intent there was to be able to double click the displayed name in the UI to rename it, or at least to have a rename_viewer (or similar) API call to do that.

[havok2063]

I think this looks good. It checks off a few boxes from #361. One thing that may be an issue is when creating viewers from within the UI on the default application, the reference and id names get assigned None, and unknown-0. E.g. if you create a default Application, then in the UI - click the Viewer Creator button - click Profile 1D (Specviz), you get a new spectrum 1d viewer but without proper ref/id names. Then other code fails when trying to reference either unknown-0 or the None reference name. The same happens for any of the other viewers. I think we could similarly map reference names onto these.

app = Application('default')
print(app.get_viewer_reference_names())
print(app.get_viewer_ids())
[None]
['unknown-0']

I think it would be useful to also have a new method that lists all available reference names, e.g, app.list_available_viewer_references(), that one could use when creating viewers programmatically. Something like viewer_registry.members but returns a list or dictionary of available options. And a similar method to create a brand new viewer by reference name, e.g. app.create_viewer("spectrum-viewer"). But perhaps these items are for a new PR?

This will improve the app.add_data_to_viewer method as well, which accepts a viewer reference name as input. Side note: that method is slightly broken now since it seems the data labels assigned to loaded data changed at some point. But that's another issue.

[pllim]

@havok2063 , thanks for pointing that out! I did not even know default has that button. I pushed a commit to address your comments. I think changing the "reference" behavior and adding programmatic viewer creation is out of scope though (feel free to open new issues for them).

As tempted as it may be, I decided not to change viewer reference set to None because I have no idea how it is being used throughout the stack and I do not wish to open that can of worms here.

Now, you should see something like this in your workflow:

>>> from jdaviz import Application
>>> app = Application('default')
>>> app
>>> # Create some viewers interactively
>>> print(app.get_viewer_reference_names())
[None, None, None]
>>> print(app.get_viewer_ids())
['CubeVizImageView-0', 'ImvizImageView-0', 'ImvizImageView-1']
>>> v = app.get_viewer_by_id('ImvizImageView-1')
>>> v
<jdaviz.configs.imviz.plugins.viewers.ImvizImageView at ...>

[pllim]

Other tests can also now take advantage of these new fixtures but that is out of scope here.

[havok2063]

@pllim The updates look good. Do we want or need the id names to be consistent e.g. cubeviz-0 vs CubeVizImageView-0? I like the simple "helper-int" syntax. I think we could probably revisit those viewer names in the dropdown list, to make them more in line with the helper tools.

I believe the viewer reference name is the thing used to actually load data into the viewers by all the helpers/parsers in Jdaviz, e.g.

jdaviz/jdaviz/configs/cubeviz/plugins/parsers.py

Line 123 in 409587a

app.add_data_to_viewer('flux-viewer', data_label)

. The viewer ids are just the names displayed on the tabs, and I "think" are purely cosmetic, as far as I know.

For use cases where a user creates a default application, and creates some viewers interactively, I think we want to retain the ability for them to access the viewer, add or remove data programmatically using app.get_viewer, app.add_data_to_viewer and app.remove_data_from_viewer. But again, all this might be out of scope for this PR, since the default application usage and programmatic API for viewers/data never quite got fleshed out.

[pllim]

@havok2063 , as far as I am concerned for Imviz, this patch allows me to manipulate sub-viewer programmatically. I don't care much how the viewer ID is named but I do slightly prefer configname-num which is more concise. Since "default" mode has no config name, I had to fallback to viewer class name. Better ideas welcome but let's be careful to not overcomplicate things.

Viewer ID is not purely cosmetic; it is the primary key to app._viewer_store. I just made it readable and hopefully more consistent here (instead of random uuid).

I agree improvements could be made on the app interface as a whole but I also agree they are out of scope here. If you are happy with this PR as-is, please approve. If not, please suggest things I should change. Thanks!

[havok2063]

@pllim that makes sense to me. This is a good improvement and don't mean to drag it out. Just some food for thought. I approve it, although I think since I'm not technically a part of the Indigo team, they may want to have 2 approvals from the more reputable JDAT folks. ;)

[pllim]

Thanks for the approval! I'll keep it open as you requested. But FWIW, you are much more reputable than I am. 😸

[ibusko]

Cool!