pyapp-kit / magicgui

build GUIs from type annotations
https://pyapp-kit.github.io/magicgui/
MIT License
362 stars 49 forks source link

optional params as checkbox with widget #645

Open tlambert03 opened 5 months ago

tlambert03 commented 5 months ago

@cmalinmayor does this for parameters that have Optional[thing]:

Screenshot 2024-04-22 at 4 17 12 PM

i like it, and we could do something similar here

hanjinliu commented 5 months ago

Hi @tlambert03 , Actually, I've already did a very similar thing in magicclass.

メディア1-output

In this example, I had to define a custom Optional type because magicgui has its own type mapping strategy for T | None type, but eventually that can be replaced to typing.Optional. I could not find the code you were refering to so I don't know how much it's already implemented. Do you think it is useful to directly port this widget to magicgui?

cmalinmayor commented 5 months ago

Linking code here for reference: Example of the spinbox: https://github.com/funkelab/motile-napari-plugin/blob/9a35d20ea28653759021de74aa495675f5c61869/src/motile_plugin/widgets/solver_params.py#L35 And the checkbox: https://github.com/funkelab/motile-napari-plugin/blob/9a35d20ea28653759021de74aa495675f5c61869/src/motile_plugin/widgets/solver_params.py#L80 My implementation seems quite similar, with a few differences. It allows default values that aren't None, while still allowing you to uncheck the box to set it to None in the backend (in the UI, it disables the linked SpinBox or in your case slider so it appears greyed out). I didn't use magicgui, so I think my code is more inspiration than an actual starting point. There is also extra stuff in that code example to allow two-way updating (e.g. you want to view a new SolverParams object, and set the spinbox and checkbox accordingly) - which presumably can be ignored for this issue.

tlambert03 commented 5 months ago

thank you both. @hanjinliu it would be great if you wanted to adapt your solution and contribute it here..

I think in terms of the GUI: what I'd want is probably something that looks like @cmalinmayor's widget: just a new checkbox on the left of the widget, without any text. When checked, the linked widget is enabled, and when unchecked, the linked widget is disabled and the default value is used. (the checkbox could have a tooltip that shows the literal default value that would be used when unchecked)

It allows default values that aren't None, while still allowing you to uncheck the box to set it to None in the backend

I think the semantics I would go for is:

I just don't think we should worry about a case where the user wants the checkbox to mean "go back to the default value". Since the end-user can always use the main widget to return the value to the default value, that is where it should be done (not using a checkbox). I can imagine a separate feature that would allow something like a right-click on the widget to return to the default value, but i think checkbox should gate "None-ness" not an implicit setting of the value to a non-null default value.

hanjinliu commented 5 months ago

Yes, it is how I implemented (I realized now that the sentence "Use default value" was not appropriate). I think I can send a quick PR that implements the feature.

In terms of the design, do you think the checkbox should be on the left side? It seems not consistent with other labeled widgets. It also makes the behavior of _unify_label_widths complicated. It would be much simpler if Type | None is converted into a LabeledWidget of (OptionalWidget of (widget for Type)), that is, [ label ][[ checkbox ][widget for Type]].