Refactoring to an abstract interface and concrete implementations

[gpdf]

I see that Issue #73 talks about the possibility of a Ginga-free implementation of the ImageWidget API using Matplotlib. At the time the original API was discussed in 2018 (see, for instance, #10) I had understood that the concept was to define an abstract interface and then, potentially, multiple implementations.

Was this a misunderstanding, or was there a decision to go with a concrete Ginga-based implementation?

Of course, one can always use duck typing to define a family of widgets with compatible APIs, rather than relying on formal subclassing.

LSST has a motivation to do another implementation (based on the Firefly library and its Jupyter extension.

Would the team be open to a refactoring that realized a design with a more abstract interface?

[pllim]

The plan is still the same, but I don't understand your definition of a "more abstract interface". The current API is supposed to be backend-agnostic. If you were to implement a different backend, you do your thing but load_fits() will always remain load_fits(). That is, when you look at core.py here, anything inside the method can be replaced but the method API should remain the same. Does this make sense?

We have to be careful to not make the interface too complicated. https://xkcd.com/927/

[gpdf]

It sounds like you are just suggesting duck typing?

Given the existence of a hypothetical Ginga alternative "FITSWizard", one would create a new package fitswizardwidgets, in which I would define an ImageWidget class in that namespace, and make its API as close to astrowidgets.ImageWidget as possible. Users would, ideally, only need to change from astrowidgets import ImageWidget to from fitswizardwidgets import ImageWidget to change from using one to the other?

The down side of this is that there's no simple CI procedure that could be used to discover divergences between the Ginga widget's API and the FITSWizard widget's API, since they are totally independent classes.

What I thought had been envisioned was having a base class that defined the interface, with concrete implementations that implemented it, e.g., class GingaWidget( ImageWidget ).

[pllim]

I guess abstract base class would work too. Maybe @mwcraig or @eteq can confirm which implementation was the original vision.

[mwcraig]

The original intent was indeed to have an abstract API which could be implemented in any number of ways. I had originally thought about bqplot as an alternative backend to ginga, but as I used the widget thought a matplotlib backend might be helpful too for non-interactive plots.

The current implementation is ginga-based primarily because it was the most complete solution available that worked in the notebook out-of-the-box, and it involved the least JavaScript.

I'm inclined to define the interface in an abstract base class rather than duck typing.

One thought we had early on was to have a factory class or function so that one could do ImageWidget(backend='ginga') and get a ginga-backed widget, or ImageWidget(backend='firefly') to get a firefly-backed widget, etc. I'm not particularly wedded to that approach, though.

I know I won't get around to actually writing a bqplot-backed version for month(s)...if you are interested in moving forward with firefly as the first non-ginga example that would be great.

As part of defining a base class we should talk a bit about which properties to make into traitlets that can easily be linked. The first candidate for that that springs to mind is the location of the center.

[gpdf]

Thanks, that is exactly what I thought I remembered. I support the ABC approach as well as the factory.

[pllim]

Not sure if this is too late for you, but I started working on this over at #126 .

[pllim]

I am not sure how to make it change backend in a way that is sane yet but maybe we can address that separately as part of #122 .

[gpdf]

@pllim I'm sorry, I didn't see your message in February.

I am still very interested in this and in implementing a new back end. I will have a look at the ABC in the next few days.

[gpdf]

I'm reading

• https://astrowidgets--126.org.readthedocs.build/en/126/api/astrowidgets.BaseImageWidget.html
• https://astrowidgets--126.org.readthedocs.build/en/126/abstract.html

[pllim]

@gpdf , glad to hear you are still interested! @astrofrog has a very different idea of how the abstract class should work on the glue-jupyter side, so now I am not sure if my design is still valid. 🤷

[gpdf]

I have some concerns myself that the design has some Ginga-isms in it that will produce awkward results with other back ends. I'll try to write up a longer set of comments later today.

[pllim]

From meeting between @mwcraig , @eteq , and myself today (2021-07-14), we have decided to have a path forward in the following way:

• Have this abstract class to be implemented in such way that any concrete implementation must fulfill certain promises in terms of functionality.
• To this end, this abstract class will also provide a set of tests that developers of the concrete implementations can run to ensure that these promises are fulfilled.
• Split out any concrete implementation from this repo into new repo(s), but still mention them here. Pick one as the "reference" concrete implementation.

p.s. Please correct me if I am wrong!

[pllim]

This is naturally solved by #162