diff --git a/magicgui/backends/_qtpy/widgets.py b/magicgui/backends/_qtpy/widgets.py index 881100311..e419ab863 100644 --- a/magicgui/backends/_qtpy/widgets.py +++ b/magicgui/backends/_qtpy/widgets.py @@ -241,9 +241,6 @@ def __init__(self): # super().__init__(QtW.QToolButton) -# CATEGORICAL - - class Container( QBaseWidget, _protocols.ContainerProtocol, _protocols.SupportsOrientation ): @@ -255,49 +252,19 @@ def __init__(self, layout="vertical"): self._layout = QtW.QVBoxLayout() self._qwidget.setLayout(self._layout) - def _mgui_get_margins(self) -> Tuple[int, int, int, int]: - m = self._layout.contentsMargins() - return m.left(), m.top(), m.right(), m.bottom() - - def _mgui_set_margins(self, margins: Tuple[int, int, int, int]) -> None: - self._layout.setContentsMargins(*margins) - - def _mgui_add_widget(self, widget: Widget): - _widget = widget.native - self._layout.addWidget(_widget) - def _mgui_insert_widget(self, position: int, widget: Widget): - _widget = widget.native - if position < 0: - position = self._mgui_count() + position + 1 - - self._layout.insertWidget(position, _widget) + self._layout.insertWidget(position, widget.native) def _mgui_remove_widget(self, widget: Widget): self._layout.removeWidget(widget.native) widget.native.setParent(None) - def _mgui_remove_index(self, position: int): - # TODO: normalize position in superclass - if position < 0: - position = self._mgui_count() + position + 1 - item = self._layout.takeAt(position) - item.widget().setParent(None) - - def _mgui_count(self) -> int: - return self._layout.count() - - def _mgui_index(self, widget: Widget) -> int: - return self._layout.indexOf(widget.native) + def _mgui_get_margins(self) -> Tuple[int, int, int, int]: + m = self._layout.contentsMargins() + return m.left(), m.top(), m.right(), m.bottom() - def _mgui_get_index(self, index: int) -> Optional[Widget]: - # We need to return a magicgui.Widget object, so this currently - # requires storing the original object as a hidden attribute. - # better way? - item = self._layout.itemAt(index) - if item: - return item.widget()._magic_widget - return None + def _mgui_set_margins(self, margins: Tuple[int, int, int, int]) -> None: + self._layout.setContentsMargins(*margins) def _mgui_set_orientation(self, value) -> None: """Set orientation, value will be 'horizontal' or 'vertical'.""" @@ -313,9 +280,6 @@ def _mgui_get_orientation(self) -> str: else: return "vertical" - def _mgui_get_native_layout(self) -> QtW.QLayout: - return self._layout - class SpinBox(QBaseRangedWidget): def __init__(self): @@ -565,7 +529,7 @@ def _mgui_get_cell(self, row: int, col: int) -> Any: return item.data(self._DATA_ROLE) widget = self._qwidget.cellWidget(row, col) if widget: - return widget._magic_widget + return getattr(widget, "_magic_widget", widget) def _mgui_set_cell(self, row: int, col: int, value: Any) -> None: """Set current value of the widget.""" diff --git a/magicgui/widgets/_bases/container_widget.py b/magicgui/widgets/_bases/container_widget.py index b7dea1313..4357bbfb0 100644 --- a/magicgui/widgets/_bases/container_widget.py +++ b/magicgui/widgets/_bases/container_widget.py @@ -81,6 +81,7 @@ def __init__( return_annotation: Any = None, **kwargs, ): + self._list: List[Widget] = [] self._return_annotation = None self._labels = labels self._layout = layout @@ -141,19 +142,11 @@ def __getitem__(self, key): # noqa: F811 if isinstance(key, str): return self.__getattr__(key) if isinstance(key, slice): - out = [] - for idx in range(*key.indices(len(self))): - item = self._widget._mgui_get_index(idx) - if item: - out.append(item) - return out + return [getattr(item, "_inner_widget", item) for item in self._list[key]] elif isinstance(key, int): - if key < 0: - key += len(self) - item = self._widget._mgui_get_index(key) - if item is None: - raise IndexError("Container index out of range") - return getattr(item, "_inner_widget", item) + item = self._list[key] + return getattr(item, "_inner_widget", item) + raise TypeError(f"list indices must be integers or slices, not {type(key)}") def index(self, value: Any, start=0, stop=9223372036854775807) -> int: """Return index of a specific widget instance (or widget name).""" @@ -172,16 +165,17 @@ def __delattr__(self, name: str): def __delitem__(self, key: Union[int, slice]): """Delete a widget by integer or slice index.""" if isinstance(key, slice): - for idx in range(*key.indices(len(self))): - self._widget._mgui_remove_index(idx) + for item in self._list[key]: + self._widget._mgui_remove_widget(item) + elif isinstance(key, int): + self._widget._mgui_remove_widget(self._list[key]) else: - if key < 0: - key += len(self) - self._widget._mgui_remove_index(key) + raise TypeError(f"list indices must be integers or slices, not {type(key)}") + del self._list[key] def __len__(self) -> int: """Return the count of widgets.""" - return self._widget._mgui_count() + return len(self._list) def __setitem__(self, key, value): """Prevent assignment by index.""" @@ -207,6 +201,11 @@ def insert(self, key: int, widget: Widget): _widget = _LabeledWidget(widget) widget.label_changed.connect(self._unify_label_widths) + self._list.insert(key, widget) + if key < 0: + key += len(self) + # NOTE: if someone has manually mucked around with self.native.layout() + # it's possible that indices will be off. self._widget._mgui_insert_widget(key, _widget) self._unify_label_widths() @@ -218,8 +217,7 @@ def _unify_label_widths(self, event=None): widest_label = max( measure(w.label) for w in self if not isinstance(w, ButtonWidget) ) - for i in range(len(self)): - w = self._widget._mgui_get_index(i) + for w in self: if hasattr(w, "label_width"): w.label_width = widest_label # type: ignore @@ -244,11 +242,6 @@ def layout(self, value): "It is not yet possible to change layout after instantiation" ) - @property - def native_layout(self): - """Return the layout widget used by the backend.""" - return self._widget._mgui_get_native_layout() - def reset_choices(self, event=None): """Reset choices for all Categorical subWidgets to the default state. @@ -288,14 +281,14 @@ def __signature__(self) -> MagicSignature: return MagicSignature(params, return_annotation=self.return_annotation) @classmethod - def from_signature(cls, sig: inspect.Signature, **kwargs) -> "Container": + def from_signature(cls, sig: inspect.Signature, **kwargs) -> Container: """Create a Container widget from an inspect.Signature object.""" return MagicSignature.from_signature(sig).to_container(**kwargs) @classmethod def from_callable( cls, obj: Callable, gui_options: Optional[dict] = None, **kwargs - ) -> "Container": + ) -> Container: """Create a Container widget from a callable object. In most cases, it will be preferable to create a ``FunctionGui`` instead. diff --git a/magicgui/widgets/_protocols.py b/magicgui/widgets/_protocols.py index 6d85d83c1..1a9adb0e0 100644 --- a/magicgui/widgets/_protocols.py +++ b/magicgui/widgets/_protocols.py @@ -359,10 +359,6 @@ class SliderWidgetProtocol(RangedWidgetProtocol, SupportsOrientation, Protocol): class ContainerProtocol(WidgetProtocol, SupportsOrientation, Protocol): """Widget that can contain other widgets.""" - @abstractmethod - def _mgui_add_widget(self, widget: "Widget") -> None: - raise NotImplementedError() - @abstractmethod def _mgui_insert_widget(self, position: int, widget: "Widget") -> None: raise NotImplementedError() @@ -371,27 +367,6 @@ def _mgui_insert_widget(self, position: int, widget: "Widget") -> None: def _mgui_remove_widget(self, widget: "Widget") -> None: raise NotImplementedError() - @abstractmethod - def _mgui_remove_index(self, position: int) -> None: - raise NotImplementedError() - - @abstractmethod - def _mgui_count(self) -> int: - raise NotImplementedError() - - @abstractmethod - def _mgui_index(self, widget: "Widget") -> int: - raise NotImplementedError() - - @abstractmethod - def _mgui_get_index(self, index: int) -> Optional[Widget]: - """(return None instead of index error).""" - raise NotImplementedError() - - @abstractmethod - def _mgui_get_native_layout(self) -> Any: - raise NotImplementedError() - @abstractmethod def _mgui_get_margins(self) -> Tuple[int, int, int, int]: raise NotImplementedError() diff --git a/tests/test_widgets.py b/tests/test_widgets.py index 59138335d..b509d6836 100644 --- a/tests/test_widgets.py +++ b/tests/test_widgets.py @@ -173,9 +173,6 @@ def test_container_widget(): del container[-1] assert not container - if use_app().backend_name == "qt": - assert container.native_layout.__class__.__name__ == "QHBoxLayout" - def test_container_label_widths(): """Test basic container functionality.""" @@ -203,8 +200,9 @@ def test_labeled_widget_container(): w1 = widgets.Label(value="hi", name="w1") w2 = widgets.Label(value="hi", name="w2") - container = widgets.Container(widgets=[w1, w2], layout="vertical") - lw = container._widget._mgui_get_index(0) + _ = widgets.Container(widgets=[w1, w2], layout="vertical") + assert w1._labeled_widget + lw = w1._labeled_widget() assert isinstance(lw, _LabeledWidget) assert lw.visible w1.hide() @@ -360,3 +358,25 @@ def f(c): assert f.c.choices == (0, 1) container.reset_choices() assert f.c.choices == (0, 1, 2) + + +def test_container_indexing_with_native_mucking(): + """Mostly make sure that the inner model isn't messed up. + + keeping indexes with a manipulated native model *may* be something to do in future. + """ + l1 = widgets.Label(name="l1") + l2 = widgets.Label(name="l2") + l3 = widgets.Label(name="l3") + c = widgets.Container(widgets=[l1, l2, l3]) + assert c[-1] == l3 + # so far they should be in sync + native = c.native.layout() + assert native.count() == len(c) + # much with native layout + native.addStretch() + # haven't changed the magicgui container + assert len(c) == 3 + assert c[-1] == l3 + # though it has changed the native model + assert native.count() == 4