Skip to content
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

JDAT-1491: Add API to return a list of human-readable viewer name #643

Closed
pllim opened this issue May 26, 2021 · 2 comments · Fixed by #722
Closed

JDAT-1491: Add API to return a list of human-readable viewer name #643

pllim opened this issue May 26, 2021 · 2 comments · Fixed by #722
Assignees
Labels
🔥 Critical
Milestone

Comments

@pllim
Copy link
Contributor

pllim commented May 26, 2021

Currently, jdaviz application stores its viewers within app._viewer_store dictionary and the data is not human-readable. As a result, we are unable to programmatically access the viewer using a human-readable identifier unless it is the default viewer already defined in a pre-configured YAML file.

https://jira.stsci.edu/browse/JDAT-1491

Also see: #175, #372

Blocks

Example workflow (exact API negotiable)

from jdaviz.imviz import Imviz

imviz = Imviz()

# Interactively load some data, create some tiles..

# Now we want to know what are the viewers available.
# We do not know these names beforehand because some of them
# were created interactively.
available_viewer_names = imviz.app.get_viewer_names()  # ['viewer-1', 'viewer-2', ...]
desired_viewer = imviz.app.get_viewer('viewer-2')

# Now we can programmatically control desired_viewer
@pllim
Copy link
Contributor Author

pllim commented May 26, 2021

A reference needs to be set here:

jdaviz/jdaviz/app.py

Lines 1094 to 1096 in 3115559

# Create the viewer item dictionary
new_viewer_item = self._create_viewer_item(
viewer=viewer)

But it cannot be done in a predictable way unless it knows what prefix to apply based on which viztool is calling it and what is the next sequence number to attach to that prefix. Let's not randomize the sequence number here with uuid and such for readability.

@eteq
Copy link
Contributor

eteq commented Jul 2, 2021

I like your API as presented, @pllim, but with one addition/clairification: for the initial configuration (before more viewers are added), the name should be the name that comes from the configuration file. Digging into things a bit it looks like the current get_viewers gets it's names from here:

def _viewer_item_by_reference(self, reference):
- one could imagine taking the logic from that method, and just returning all the keys that correspond to items that are viewer subclasses.

TBH, I don't understand why _viewer_store's keys are UUIDs anyway - it seems more logical to me to just use human-readable names instead of UUIDs and dispense completely with the IDs separate from the references. But someone else (@nmearl, @astrofrog , @maartenbreddels ? ) might have insight that I'm missing. Regardless, I agree we want a get_viewer_names that tells you what get_viewer would accept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔥 Critical
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants