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

rewrite: proper widget protocols & signature objects #43

Merged
merged 75 commits into from
Dec 10, 2020

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Aug 11, 2020

Updated 12/7/2020 after a lot of changes

First off, I apologize for a PR that literally changes every line in the codebase 😂 ... but there was no other way!
This PR introduces proper "interfaces" that backend apps need to implement, completely separating any Qt code (while making it much clearer what would be required of a new backend), unifying the public API, and allowing better composability of widgets/objects. It also formalizes a close/natural connection between individual widget objects and inspect.Parameter objects; and between "container" objects (collections of widgets) and an inspect.Signature objects. I still need to port over some of the features (and update/write tests), but here's roughly how this is organized for anyone wanting to take a look:

  • all interfaces are declared in magicgui/bases.pymagicgui.widgets._protocols, as typing.Protocols, where the abstract methods that backends must implement are prefixed with _mg_ (e.g. _mg_get_value). They fall into subcategories based on their "capabilities"

    1. WidgetProtocol (show/hide/enable/get_parent/get_native)
      1. ValueWidgetProtocol (get/set/on_change)
        • ButtonWidgetProtocol (text)
        • RangedWidgetProtocol (min/max/step)
          • SliderWidgetProtocol (orientation)
        • CategoricalWidgetProtocol (get/set choices)
      2. ContainerProtocol (list-like container for other widgets)
    2. BaseApplicationBackend (similar to the vispy app)
  • Backends implement those protocols using their own native widgets and stuff. I've been using composition instead of inheritance (since multiple inheritance is tricky in Qt), but either is technically fine. See magicgui.backends._qtpy.widgets for examples of backend implementations.

  • classes defined in magicgui.widgets._bases then wrap backend widget implementations (they expect a "widget_type") in their constructors, and define the main public api.

  • classes defined in magicgui.widgets._concrete (and exported at magicgui.widgets) are the "actual" widgets that someone might use to just directly "create a widget". They basically just get a backend widget and use it to instantiate one of the widgets from magicgui.widgets._bases. This now means that someone could create a widget directly with magicgui.widgets.LineEdit(default='Some String')

  • the magicgui.widgets.Containerobject wraps a backend container implementation and itself implements the MutableSequence interface, so adding widgets to a layout just feels like appending/inserting to a list.

    • It provides convenience classmethods (to_signature & from_signature) that convert to and from and inspect.Signature object. (technically, it uses a MagicSignature subclass that puts some of the gui options inside the type as an Annotated type.)
  • the magicgui.Application object wraps the backend app, and is patterned after vispy.use_app.

  • The @magicgui decorator itself now just returns a special case of a Container called a FunctionGui, that wraps an Application and Container object, and binds the signature to a __call__ method... (a nice bonus is that inspect.signature(func) still works just fine on a function decorated with @magicgui... and the decorated function is still functionally equivalent to the non-decorated function)

  • mapping between types -> widgets is done in type_map.py.

@HagaiHargil
Copy link
Contributor

Looks great, well done! I really think you should sell this package to a broader audience - I use it all the time for my work, independently of napari. I believe it elegantly solves an issue many data scientists face on a daily basis.

Copy link
Contributor

@sofroniewn sofroniewn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on all accounts: ipywidgets seems like a good API to mimic (and I'll do a little more research to see if there are other strong contenders in the python ecosystem). I was actually just yesterday playing a bit with ipywidgets to see how hard it would be to create a backend for it. The magicgui decorator is in fact quite similar to the ipywidgets.interact function, so I'm hoping that it should be very doable.

That's great! I see ipywidgets here as analogous to qtpy and that having a magicgui backend for it would be incredibly powerful. Given that I don't think we need to absolutely conform to the ipywidgets API in the same way that we don't conform to the qtpy API, so I would be fine with Slider instead of IntSlider if we wanted, but I'm just glad we're thinking about it and the ipywidgets.interact function too, which you're right is highly related.

One could imagine other backends too and even moving towards an intermediate json-schema like representation if we wanted. Today someone else pointed me towards https://github.com/rjsf-team/react-jsonschema-form, which is a different use case, but a related idea. I'm also reminded of the @freeman-lab creation https://github.com/freeman-lab/control-panel from a while back!

All this to say, I really like the new direction. I think it opens up lots of possibility for magicGui to be used well beyond napari too!

I'm approving now too. We should probably fix the log slider example before merge, but everything else I'm good with!

@tlambert03
Copy link
Member Author

Looks great, well done! I really think you should sell this package to a broader audience - I use it all the time for my work, independently of napari. I believe it elegantly solves an issue many data scientists face on a daily basis.

Thanks, and thanks a lot for taking the time to have a look. If you have any non-napari examples that you'd consider contributing to the examples, I'd happily include them in the new docs i'm working on (it's based on jupyter book and show be able to show the widget result inline for a given bit of code)

@tlambert03
Copy link
Member Author

I don't think we need to absolutely conform to the ipywidgets API in the same way that we don't conform to the qtpy API, so I would be fine with Slider instead of IntSlider if we wanted, but I'm just glad we're thinking about it and the ipywidgets.interact function too

great. maybe I'll address that it a followup PR (but before this actually gets released), just to simplify the diffs moving forward.

All this to say, I really like the new direction. I think it opens up lots of possibility for magicGui to be used well beyond napari too!

Glad you like it. I hope it does open up some avenues... just starting to chat with @oeway as well about possibly making a backend that might work for imjoy too

We should probably fix the log slider example before merge, but everything else I'm good with!

cool! There's always more to be done, so I'm thinking I'll take one last look through, and then merge so the changes can go back to being incremental. If someone were to check out master at this point it should be very useable.

Thanks again everyone for dealing with a horrifically large PR.

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

Successfully merging this pull request may close these issues.

5 participants