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

Improve matplotlib example by implementing abstraction #594

Open
larsoner opened this issue Oct 5, 2023 · 8 comments
Open

Improve matplotlib example by implementing abstraction #594

larsoner opened this issue Oct 5, 2023 · 8 comments

Comments

@larsoner
Copy link

larsoner commented Oct 5, 2023

When I opened the matplotlib example I hoped the example would show how to embed a matplotlib figure in both Qt and Jupyter/ipywidgets backends seamlessly rather than showing just how to do it in Qt. One could triage between the Qt f.native.layout().addWidget and notebook equivalent, but that's not as nice given the goal here is abstraction.

I haven't looked at the internals of magicgui yet, but I'm hoping there is a clean way to get a backend-agnostic Matplotlib figure widget. My quick/half baked idea is that something like the following could work on both backends:

class MatplotlibFigureWidget(magicgui.widgets.bases.Something):
    def __init__(...):
        from matplotlib.figure import Figure
        self.figure = Figure(figsize=(width, height), dpi=dpi)
        if backend == "notebook":
            from matplotlib.backends.backend_nbagg import FigureCanvas
        else:
            from matplotlib.backends.backend_qt5agg import FigureCanvas
        FigureCanvas(self.figure)
        ...  # stuff like setting size policy etc. if needed for each backend

mpl_widget = MatplotlibFigureWidget()
ax = mpl_widget.figure.add_subplot(111)
(line,) = ax.plot(data[123])
container = magicgui.widgets.Container([mpl_widget])
container.show()

Does this idea seem reasonable? If so, any quick hint for adjustments to the above or what to read to do it correctly would be appreciated, otherwise I'm happy to dive a bit further into the internals to figure it out.

The end goal would be improving that example to show people how to implement a new Qt-and-notebook compatible widget, since matplotlib is a nice example.

Why I want this

In MNE-Python we already have a home-grown Qt/notebook abstraction for a GUI that isn't always pretty but at least works similarly for both:

Qt notebook

I think most of this is translatable to magicgui without too much work. The ugly part at the moment would be the matplotlib plot at the bottom and the PyVista plot at the top, hence why I want to figure out the best way to implement this abstraction.

@tlambert03
Copy link
Member

Thanks for opening this @larsoner, and thanks for the link to mne-tools/mne-python#11990

first off: i absolutely recognize the truck factor concern 😂 I don't want this project to be that way of course. napari does depend on it, and many plugins there do as well, so there's at least a little additional awareness/involvement from napari core developers. it's definitely true that I don't have a ton of active support, but remain dedicated to maintaining it while it has demonstrated usage.

I would be very happy to work on adding accommodations that would help MNE-python inasmuch as they generally overlap with the goals here, which is broadly, 1) to autogenerate guis based on types and 2) to abstract the gui backend.
With regards to integration with matplotlib, we certainly fall short of goal (2)!

Given its prominence, I would be happy to include a matplotlib figure adapter as a special case here (provided we don't add it as a dependency). And yes, your MatplotlibFigureWidget proposal seems very reasonable. 👍

If so, any quick hint for adjustments to the above or what to read to do it correctly would be appreciated, otherwise I'm happy to dive a bit further into the internals to figure it out.

give me some time to have a quick look to see where I think this would plug in best. (mostly, I need to make myself more aware of how matplotlib interacts with jupyter widgets). I'm at a conference this week, so might be a bit delayed.

@tlambert03
Copy link
Member

also happy to have a zoom sometime next week if you'd like to more generally discuss magicgui and mne

@larsoner
Copy link
Author

larsoner commented Oct 5, 2023

give me some time to have a quick look to see where I think this would plug in best. (mostly, I need to make myself more aware of how matplotlib interacts with jupyter widgets)

We use ipympl which gives a Canvas object that subclasses DOMWidget:

https://github.com/matplotlib/ipympl/blob/238d7965c52f3fe8de02242c096714e064847819/ipympl/backend_nbagg.py#L180

I would be very happy to work on adding accommodations that would help MNE-python inasmuch as they generally overlap with the goals here, which is broadly, 1) to autogenerate guis based on types and 2) to abstract the gui backend. With regards to integration with matplotlib, we certainly fall short of goal (2)!

Yes for MNE we probably wouldn't use the type stuff (we don't use type hints at all currently 😱 ) but magicgui's abstraction could ideally replace 3-4k lines of abstraction at our end with cleaner code.

In a few hours of work today I made this in ~150 lines (MNE's current Qt interface on the left, and just some layouts with no meaningful interaction in the middle and right using magicgui) including a matplotlib plot and PyVista plot using conditionals to embed in the .native of each backend for now:

MNE magic-Qt magic-notebook

also happy to have a zoom sometime next week if you'd like to more generally discuss magicgui and mne

Thanks for the offer! I don't think it's necessary yet because even given what you have on main we could already make quite decent progress. I opened #593 because it's something that was obviously missing but we'll see if other stuff comes up as I find time to work on this.

@tlambert03
Copy link
Member

awesome 😎 very promising.

I would definitely welcome a MatplotlibFigureWidget PR if you wanted to contribute it (and, if you don't, i'm happy to work on it as well when I have a moment). there is a rough general guide to adding a new widget here: https://pyapp-kit.github.io/magicgui/CONTRIBUTING/#adding-a-widget
that includes the important modules to modify (and, as mentioned in those docs, #483 shows a minimal PR to add a widget.. though that specific pr lacks support for the ipywidgets backend)

next week i'll get on #593

@larsoner
Copy link
Author

larsoner commented Oct 5, 2023

No rush from my end! I'll let you know if I keep working on either this or #593 -- can you do the same if you start one so I don't duplicate effort? And thanks for the contrib link, that should help

@tlambert03
Copy link
Member

will do, thanks!

@Czaki
Copy link
Contributor

Czaki commented Oct 10, 2023

so there's at least a little additional awareness/involvement from napari core developers.

I could confirm. Maybe napari core devs are not currently involved in the development of new features, but we are ready to support maintenance if there is such a need.

@tlambert03
Copy link
Member

thanks @Czaki, that's very much appreciated

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

No branches or pull requests

3 participants