Imviz astrowidgets API to work on tiled viewers

[pllim]

Currently viewer-1 is hardcoded everywhere. Now that #722 is merged, user should be able to use astrowidgets API on tiled viewers.

This should only be worked on after the following remaining astrowidgets API are implemented:

• Imviz astrowidgets API MVP: zoom_level, zoom #715
• Imviz astrowidgets API MVP: stretch_options, stretch #716
• Imviz astrowidgets API MVP: autocut_options, cuts #717
• Imviz astrowidgets API MVP: colormap_options, set_colormap #718
• Imviz astrowidgets API MVP: save #719

🐱

[pllim]

@eteq suggested maybe we should move the API from

jdaviz/jdaviz/configs/imviz/helper.py

Line 27 in d0c2ce0

class Imviz(ConfigHelper):

to

jdaviz/jdaviz/configs/imviz/plugins/viewers.py

Line 13 in d0c2ce0

class ImvizImageView(BqplotImageView):

I think that makes sense, but do we have enough time left to do this? I think it needs at least half a sprint, uninterrupted. But then again, it is better to get this done before the first release.

[pllim]

Before I forget, we might need to keep markers API in Imviz, as markers are "global" and not tied to a specific viewer. But doing that might also break the universal clause of astrowidgets philosophy?

[pllim]

See #827 for a compromise. I think this is the first second solution suggested by @eteq , among three.

1. Promote to astrowidgets the generic idea of a "viewer" - e.g., a keyword that a backend can ignore or not depending on whether it has the concept of multiple viewers
2. Give our helper class a state along the lines of "current widget" - then the astrowidget api applies to that and you have to set it to determine the oprations it applies to.
3. The astrowidgets API could be per viewer. I.e., the helper would have something like imviz.get_api('viewer1') which then gives back an astrowidget-compatible API. Or better yet (although maybe harder?) imviz.get_viewer(...).astrowidget_api_call.

[eteq]

@pllim and I discussed this a bit out-of-band after thinking a bit more about #827. So with that in mind I wanted to make a case for option 3 from above:

The astrowidgets API is pretty focused on one "viewer" - and I think keeping it that way will make it easier to write fairly generic code using astrowidgets and simplify implementation of backends for it. So that argues pretty strongly against 1.

My main issue with 2 is that it leads to workflows more or less like this:

imviz.set_active_viewer('imviz-1')
imviz.do_something_in_code(...)

imviz.set_active_viewer('imviz-2')
imviz.do_something_in_code(...)

... lots of other code in a notebook, followed by  user does some UI interaction where they click on viewer 1 ...

imviz.do_something_in_code_intended_for_viewer1(...)

that is, the user has to mentally keep track of what the "code" and "UI" are doing separately. Whereas in case 3 they do:

imviz.get_viewer('imviz-1').do_something_in_code(...)

imviz.get_viewer('imviz-2').do_something_in_code(...)

... lots of other code in a notebook, followed by  user does some UI interaction where they click on viewer 1 ...

imviz.get_viewer('imviz-1').do_something_in_code_intended_for_viewer1(...)

there's no way to get confused, because you have to choose the right viewer explicitly.

[eteq]

Oh, and @pllim pointed out there's some rough edges option 3 has to work around, most notably the fact that some of the astrowidget API choices add marker data to glue which then would be visible in the other viewers. But I personally think that's ok because the individual viewers can still keep track of which are "their" markers. And it's ok if users can see those in the UI, just so long as that doesn't "leak" into astrowidgets code.

[pllim]

Thanks for a nice summary, @eteq !

I also want to make it clear that I have strong opinion that we should decide on Option 2 or Option 3 before MVP release. I am very much against Option 2 first and then Option 3 refactor, because that introduces maintenance and user experience nightmares.