Skip to content

Commit

Permalink
Fix duplicate choice names in ipynb Dropdown, add Separator support
Browse files Browse the repository at this point in the history
- Fix #639
- Note that `_mgui_set_choice()` is not used
  • Loading branch information
qin-yu committed Mar 18, 2024
1 parent 2fe9693 commit 2e7b7b3
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 12 deletions.
7 changes: 5 additions & 2 deletions src/magicgui/backends/_ipynb/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
) from e


from magicgui.types import Separator

Check warning on line 15 in src/magicgui/backends/_ipynb/widgets.py

View check run for this annotation

Codecov / codecov/patch

src/magicgui/backends/_ipynb/widgets.py#L15

Added line #L15 was not covered by tests
from magicgui.widgets import protocols

if TYPE_CHECKING:
Expand Down Expand Up @@ -219,7 +220,9 @@ def _mgui_get_choices(self) -> tuple[tuple[str, Any]]:

def _mgui_set_choices(self, choices: Iterable[tuple[str, Any]]) -> None:
"""Set available choices."""
self._ipywidget.options = choices
options = [item for item in choices if item[1] is not Separator]
options = list(dict(options).items()) # remove duplicate names, last data used
self._ipywidget.options = options

Check warning on line 225 in src/magicgui/backends/_ipynb/widgets.py

View check run for this annotation

Codecov / codecov/patch

src/magicgui/backends/_ipynb/widgets.py#L223-L225

Added lines #L223 - L225 were not covered by tests

def _mgui_del_choice(self, choice_name: str) -> None:
"""Delete a choice."""
Expand Down Expand Up @@ -248,7 +251,7 @@ def _mgui_get_current_choice(self) -> str:

def _mgui_set_choice(self, choice_name: str, data: Any) -> None:
"""Set the data associated with a choice."""
self._ipywidget.options = (*self._ipywidget.options, (choice_name, data))
pass


class _IPySupportsText(protocols.SupportsText):
Expand Down
28 changes: 18 additions & 10 deletions tests/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1067,9 +1067,11 @@ def test_separator_singleton():
assert sep1 is sep2


def test_separator():
def test_separator(backend: str):
from magicgui.types import Separator

use_app(backend)

sep = [[Separator] * (i + 1) for i in range(4)]
a = [1, 2, *sep[0], 4, *sep[1], 6, 7, *sep[2], 9, *sep[3], 11, 12, *sep[0], 14, *sep[0]]
b = [1, 2, *sep[0], 4, *sep[1], 6, 7, *sep[2], 9, *sep[3], 6, 7, *sep[0], 9, *sep[0]]
Expand All @@ -1084,21 +1086,27 @@ def test_separator():
def widget_with_separator(a=a[0], b=b[0]):
return

def get_all_itemdata(combo_box):
return [combo_box.itemData(index) for index in range(combo_box.count())]
if backend == "qt":
def get_all_itemdata(combo_box):
return [combo_box.itemData(index) for index in range(combo_box.count())]

# Count returns the number of all items including separator items
assert widget_with_separator.a.native.count() == 21
assert widget_with_separator.b.native.count() == 18

# Separator singletons themselves are used as separator item data
assert get_all_itemdata(widget_with_separator.a.native) == a
assert get_all_itemdata(widget_with_separator.b.native) == b2

# Count returns the number of all items including separator items
assert widget_with_separator.a.native.count() == 21
assert widget_with_separator.b.native.count() == 18
if backend == "ipynb":
# Separator singletons themselves are used as separator item data
assert widget_with_separator.a.options['choices'] == a
assert widget_with_separator.b.options['choices'] == b # non-separators are not unique

# Choices only returns unique, non-separator items
assert widget_with_separator.a.choices == (1, 2, 4, 6, 7, 9, 11, 12, 14)
assert widget_with_separator.b.choices == (1, 2, 4, 6, 7, 9)

# Separator singletons themselves are used as separator item data
assert get_all_itemdata(widget_with_separator.a.native) == a
assert get_all_itemdata(widget_with_separator.b.native) == b2


def test_float_slider_readout():
sld = widgets.FloatSlider(value=4, min=0.5, max=10.5)
Expand Down

0 comments on commit 2e7b7b3

Please sign in to comment.