magicgui icon indicating copy to clipboard operation
magicgui copied to clipboard

optional params as checkbox with widget

Open tlambert03 opened this issue 1 year ago • 4 comments

@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

tlambert03 avatar Apr 22 '24 20:04 tlambert03

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?

hanjinliu avatar Apr 29 '24 06:04 hanjinliu

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.

cmalinmayor avatar Apr 29 '24 14:04 cmalinmayor

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:

  • if a parameter is annotated as x: int | None = None, then you get a value widget (slider or spinbox) and the checkbox is unchecked by default, and the value is None. checking the box provides the value
  • if a parameter is annotated as x: int | None = 10, then you get a value widget (slider or spinbox) and the checkbox is checked by default, and the value is 10. unchecking the box returns None
  • (parameters that don't have None as an optional value don't have checkboxes)

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.

tlambert03 avatar Apr 29 '24 15:04 tlambert03

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]].

hanjinliu avatar Apr 30 '24 11:04 hanjinliu