maxfordham / ipyautoui

create ipywidgets user input form pydantic model or jsonschema
https://maxfordham.github.io/ipyautoui/
42 stars 4 forks source link

Fix bugs in setting FileChooser value #323

Closed mwcraig closed 3 months ago

mwcraig commented 4 months ago

This pull requests fixes a couple of issues with. setting the value of a FileChooser widget.

There were two separate issues, though they had similar fixes.

The first issue was that setting the value to a file that did not exist was impossible. This code demonstrates that bug:

import tempfile
from pathlib import Path

from ipyautoui.custom.filechooser import FileChooser

fc = FileChooser()

with tempfile.TemporaryDirectory() as tmpdirname:
    tmp_path = Path(tmpdirname)
    fc.value = tmp_path / "does_not_exist.txt"
    assert fc.selected_filename == "does_not_exist.txt"  # Fails because fc.selected_filename is ""

The root cause in this case is this line, where I think the correct behavior is to set both the path and name.

The second issue is that setting the FileChooser value to an existing directory after having set the FileChooser file that exists fails to update the selected_filename, as demonstrated by this code:

import tempfile
from pathlib import Path

from ipyautoui.custom.filechooser import FileChooser

fc = FileChooser()

with tempfile.TemporaryDirectory() as tmpdirname:
    tmp_path = Path(tmpdirname)
    new_file = tmp_path / "does_not_exist.txt"
    new_file.touch()
    fc.value = new_file
    assert fc.selected_filename == "does_not_exist.txt"  # Work since the file exists

    new_dir_path = tmp_path / "new_directory/"
    new_dir_path.mkdir()
    fc.value = new_dir_path
    assert fc.selected_filename == ""  # Fails because fc.selected_filename is still "does_not_exist.txt"

The root cause of this issue is the handling of this case, where "" should be passed as the file name instead of None to indicate there is no file.

jgunstone commented 3 months ago

hi @mwcraig - sorry, this one slipped past me - all looks good, we'll have a quick look and get it merged asap - thanks very much :)

mwcraig commented 3 months ago

No worries about the delay -- I've had plenty of other stuff to work on 😀