Optional types should map to the same widgets as non-optional and be bound to the default None

[jni]

Describe the bug

Annotating a function argument with pathlib.Path gives me a native file selection widget, which is 👍! Changing it to Optional[pathlib.Path] = None (a) fails to find the file widget anymore, and (b) gives an unbound argument error when run:

/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/widgets/_bases/widget.py:66: FutureWarning:

EmptyWidget.__init__() got unexpected keyword arguments {'mode'}.
In the future this will raise an exception

  warnings.warn(
Traceback (most recent call last):
  File "/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/widgets/_bases/value_widget.py", line 44, in <lambda>
    lambda *x: self.changed(value=x[0] if x else None)
  File "/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/events.py", line 603, in __call__
    self._invoke_callback(cb, event)
  File "/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/events.py", line 621, in _invoke_callback
    _handle_exception(
  File "/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/events.py", line 619, in _invoke_callback
    cb(event)
  File "/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/widgets/_function_gui.py", line 179, in <lambda>
    self._call_button.changed.connect(lambda e: self.__call__())
  File "/Users/jni/conda/envs/all2/lib/python3.9/site-packages/magicgui/widgets/_function_gui.py", line 252, in __call__
    raise TypeError(msg) from None
TypeError: missing a required argument: 'fn' in call to 'widget(fn: Optional[pathlib.Path])'.
To avoid this error, you can bind a value or callback to the parameter:

    widget.fn.bind(value)

Or use the 'bind' option in the magicgui decorator:

    @magicgui(fn={'bind': value})
    def widget(fn: Optional[pathlib.Path]): ...

To Reproduce

from typing import Optional
import pathlib
from magicgui import magicgui


@magicgui(fn={'mode': 'r'}, call_button='Run')
def widget(fn: Optional[pathlib.Path] = None):
    print(fn)


widget.show(run=True)

Expected behavior I would have expected the file widget to appear, and the function to run with None as the corresponding argument if no file is selected.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

• OS: [e.g. Windows, Mac] Mac
• backend: [e.g. Qt 5.13.0] Qt
• magicgui version [e.g. 0.1.0] 0.2.6

[tlambert03]

good one!

[tlambert03]

while pretty straightforward for FileEdit and LineEdit, this is actually kinda tricky to do in a more general way, and comes back to the question posed in https://github.com/napari/magicgui/issues/30 (about how to generally interpret the lack of input in a widget). I'm wondering if ValueWidgets should perhaps have a nullable[=False] attribute that signifies that they accept null values. LineEdits and FileEdits would then return None instead of the empty string when empty, things like SpinBoxes and ComboBoxes would gain a new special "value" that represents the absence of input ... which would be converted to None in python. And that option would be "activated" by using Optional[type] ... thoughts @jni?

edit: something like x: str = None would also signify that lack of input should be considered None instead of ""

[jni]

@tlambert03 to clarify: your question is about widgets and how they deal with nullable, not about the typing, right? And I see that with things like sliders etc it's essentially impossible without e.g. adding a "None" tick box next to it — is that what you're thinking?

At any rate imho I think it's fine to punt on the harder widgets and start with the obvious ones (combobox, radiobuttons). I actually don't think that the empty string in the path/lineedit should be None, that sounds like something that will bite us in the butt. 😂

[jni]

actually don't think that the empty string in the path/lineedit should be None, that sounds like something that will bite us in the butt

@tlambert03 and I thought about this some more and decided that actually it would be ok — since as far as we know no filesystem allows the empty string to be a valid path.

[aeisenbarth]

I would be very interested if this was working, especially since paths don't have an inherent null value (Path("") would resolve to the working directory, whereas for pure strings you could interpret "" as null). I have to resort to using Path("None") to set paths to null and check against Path("None").expand_user().absolute().

What is the current status on this? The documentation inherits the nullable argument for all value widgets, but it seems to be working only for categorical widgets, no matter whether I pass the annotation as Path or Optional[Path].

Thanks!

[tlambert03]

Path (and FileEdit) is a little odd in that it doesn't actually currently inherit from ValueWidget (rather, it's a container). It probably could/should inherit from it (it certainly implements the protocol)... So, something like what was done for Table in #360 should be done for Path, then making it nullable would be easier

[l-spiecker]

I'm new here, so maybe i missed something: I would be very interested also in a more general implementation for Optional for all types. My proposed gui change would be to simply replace the label of a widget by a checkbox.

[X] my_optional_option [__________________]

Visually we could then set the widget itself disabled, until the user clicks on the widget or checks the checkbox. Don't know if this is complicated with ipywidgets, but it would be easy for qt.

Especially when bringing a cli to a gui, this happens very often.

[tlambert03]

Hi @l-spiecker, thanks for your note.

I think on the level of mapping types to widgets, that feels a bit too much to me. It kind of conflates the concepts of "editability" and enablement with that of an optional value (something that can either be None or an instance of the type).

We implemented support for the latter (nullable values ) in https://github.com/pyapp-kit/magicgui/pull/227 so that when you annotate something as Optional[type], then the widget can make accommodations to render a "nullable" value.

What you're proposing (an additional checkbox to toggle the enablement of a widget) does sound like a potentially useful feature! but I think we need something a bit more explicit to enable that on top of Optional[thing]