Fix regression from 2.6.2: weight can be float (#827)

* Document in typing annotations around code * Cover by tests Fix: #826 Co-authored-by: Aleksei Stepanov <alekseis@nvidia.com>
urwid · Feb 21, 2024 · 36bf037 · 36bf037
1 parent c9655d4
commit 36bf037
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 48 deletions.
diff --git a/tests/test_columns.py b/tests/test_columns.py
@@ -743,6 +743,10 @@ def test_common(self):
             c.contents[0:0] = [(t3, ("given", 10, False)), (t2, ("weight", 1, False))]
             c.contents.insert(3, (t1, ("pack", None, False)))
             self.assertEqual(c.focus_position, 2)
+
+        with self.subTest("Contents change validation"):
+            c.contents.clear()
+
             self.assertRaises(urwid.ColumnsError, lambda: c.contents.append(t1))
             self.assertRaises(urwid.ColumnsError, lambda: c.contents.append((t1, None)))
             self.assertRaises(urwid.ColumnsError, lambda: c.contents.append((t1, "given")))
@@ -755,6 +759,10 @@ def test_common(self):
             self.assertRaises(urwid.ColumnsError, lambda: c.contents.append((t1, ("given", (), False))))
             # Incorrect size
             self.assertRaises(urwid.ColumnsError, lambda: c.contents.append((t1, ("given", -1, False))))
+            # Float and int weight accepted
+            c.contents.append((t1, ("weight", 1, False)))
+            c.contents.append((t2, ("weight", 0.5, False)))
+            self.assertEqual(("one   two",), c.render(()).decoded_text)
 
     def test_focus_position(self):
         t1 = urwid.Text("one")

diff --git a/tests/test_pile.py b/tests/test_pile.py
@@ -405,6 +405,9 @@ def test_common(self):
             p.contents[0:0] = [(t3, ("pack", None)), (t2, ("pack", None))]
             p.contents.insert(3, (t1, ("pack", None)))
             self.assertEqual(p.focus_position, 2)
+
+        with self.subTest("Contents change validation"):
+            p.contents.clear()
             self.assertRaises(urwid.PileError, lambda: p.contents.append(t1))
             self.assertRaises(urwid.PileError, lambda: p.contents.append((t1, None)))
             self.assertRaises(urwid.PileError, lambda: p.contents.append((t1, "given")))
@@ -415,7 +418,10 @@ def test_common(self):
             self.assertRaises(urwid.PileError, lambda: p.contents.append((t1, ("given", ()))))
             # incorrect size
             self.assertRaises(urwid.PileError, lambda: p.contents.append((t1, ("given", -1))))
-
+            # Float and int weight accepted
+            p.contents.append((t1, ("weight", 1)))
+            p.contents.append((t2, ("weight", 0.5)))
+            self.assertEqual(("one", "two"), p.render((3,)).decoded_text)
 
     def test_focus_position(self):
         t1 = urwid.Text("one")

diff --git a/urwid/widget/columns.py b/urwid/widget/columns.py
@@ -195,7 +195,8 @@ def __init__(
         widget_list: Iterable[
             Widget
             | tuple[Literal["pack", WHSettings.PACK] | int, Widget]
-            | tuple[Literal["weight", "given", WHSettings.WEIGHT, WHSettings.GIVEN], int, Widget]
+            | tuple[Literal["given", WHSettings.GIVEN], int, Widget]
+            | tuple[Literal["weight", WHSettings.WEIGHT], int | float, Widget]
         ],
         dividechars: int = 0,
         focus_column: int | Widget | None = None,
@@ -216,13 +217,11 @@ def __init__(
         *widget_list* may also contain tuples such as:
 
         (*given_width*, *widget*)
-            make this column *given_width* screen columns wide, where *given_width*
-            is an int
+            make this column *given_width* screen columns wide, where *given_width* is an int
         (``'pack'``, *widget*)
             call :meth:`pack() <Widget.pack>` to calculate the width of this column
         (``'weight'``, *weight*, *widget*)
-            give this column a relative *weight* (number) to calculate its width from the
-            screen columns remaining
+            give this column a relative *weight* (number) to calculate its width from th screen columns remaining
 
         Widgets not in a tuple are the same as (``'weight'``, ``1``, *widget*)
 
@@ -241,7 +240,8 @@ def __init__(
         self._contents: MonitoredFocusList[
             Widget,
             tuple[Literal[WHSettings.PACK], None, bool]
-            | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int, bool],
+            | tuple[Literal[WHSettings.GIVEN], int, bool]
+            | tuple[Literal[WHSettings.WEIGHT], int | float, bool],
         ] = MonitoredFocusList()
         self._contents.set_modified_callback(self._contents_modified)
         self._contents.set_focus_changed_callback(lambda f: self._invalidate())
@@ -294,7 +294,7 @@ def _validate_contents_modified(self, slc, new_items) -> None:
                 if any(
                     (
                         t not in {WHSettings.PACK, WHSettings.GIVEN, WHSettings.WEIGHT},
-                        (n is not None and (not isinstance(n, int) or n < 0)),
+                        (n is not None and (not isinstance(n, (int, float)) or n < 0)),
                         not isinstance(b, bool),
                     )
                 ):
@@ -443,7 +443,9 @@ def contents(
         self,
     ) -> MonitoredFocusList[
         Widget,
-        tuple[Literal[WHSettings.PACK], None, bool] | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int, bool],
+        tuple[Literal[WHSettings.PACK], None, bool]
+        | tuple[Literal[WHSettings.GIVEN], int, bool]
+        | tuple[Literal[WHSettings.WEIGHT], int | float, bool],
     ]:
         """
         The contents of this Columns as a list of `(widget, options)` tuples.
@@ -460,7 +462,8 @@ def contents(
         c: Sequence[
             Widget,
             tuple[Literal[WHSettings.PACK], None, bool]
-            | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int, bool],
+            | tuple[Literal[WHSettings.GIVEN], int, bool]
+            | tuple[Literal[WHSettings.WEIGHT], int | float, bool],
         ],
     ) -> None:
         self._contents[:] = c
@@ -470,9 +473,13 @@ def options(
         width_type: Literal[
             "pack", "given", "weight", WHSettings.PACK, WHSettings.GIVEN, WHSettings.WEIGHT
         ] = WHSettings.WEIGHT,
-        width_amount: int | None = 1,
+        width_amount: int | float | None = 1,  # noqa: PYI041  # provide explicit for IDEs
         box_widget: bool = False,
-    ) -> tuple[Literal[WHSettings.PACK], None, bool] | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int, bool]:
+    ) -> (
+        tuple[Literal[WHSettings.PACK], None, bool]
+        | tuple[Literal[WHSettings.GIVEN], int, bool]
+        | tuple[Literal[WHSettings.WEIGHT], int | float, bool]
+    ):
         """
         Return a new options tuple for use in a Pile's .contents list.
 
@@ -495,10 +502,10 @@ def options(
         :type box_widget: bool
         """
         if width_type == WHSettings.PACK:
-            width_amount = None
-        if width_type not in {WHSettings.PACK, WHSettings.GIVEN, WHSettings.WEIGHT}:
-            raise ColumnsError(f"invalid width_type: {width_type!r}")
-        return (WHSettings(width_type), width_amount, box_widget)
+            return (WHSettings.PACK, None, box_widget)
+        if width_type in {WHSettings.GIVEN, WHSettings.WEIGHT} and width_amount is not None:
+            return (WHSettings(width_type), width_amount, box_widget)
+        raise ColumnsError(f"invalid combination: width_type={width_type!r}, width_amount={width_amount!r}")
 
     def _invalidate(self) -> None:
         self._cache_maxcol = None

diff --git a/urwid/widget/constants.py b/urwid/widget/constants.py
@@ -222,21 +222,23 @@ def normalize_width(
 
 @typing.overload
 def normalize_width(
-    width: tuple[Literal["weight", WHSettings.WEIGHT], int],
+    width: tuple[Literal["weight", WHSettings.WEIGHT], int | float],
     err: type[BaseException],
-) -> tuple[Literal[WHSettings.WEIGHT], int]: ...
+) -> tuple[Literal[WHSettings.WEIGHT], int | float]: ...
 
 
 def normalize_width(
     width: (
         Literal["clip", "pack", WHSettings.CLIP, WHSettings.PACK]
         | int
-        | tuple[Literal["relative", "weight", WHSettings.RELATIVE, WHSettings.WEIGHT], int]
+        | tuple[Literal["relative", WHSettings.RELATIVE], int]
+        | tuple[Literal["weight", WHSettings.WEIGHT], int | float]
     ),
     err: type[BaseException],
 ) -> (
     tuple[Literal[WHSettings.CLIP, WHSettings.PACK], None]
-    | tuple[Literal[WHSettings.GIVEN, WHSettings.RELATIVE, WHSettings.WEIGHT], int]
+    | tuple[Literal[WHSettings.GIVEN, WHSettings.RELATIVE], int]
+    | tuple[Literal[WHSettings.WEIGHT], int | float]
 ):
     """
     Split width into (width_type, width_amount).  Raise exception err
@@ -282,8 +284,8 @@ def simplify_width(
 @typing.overload
 def simplify_width(
     width_type: Literal["weight", WHSettings.WEIGHT],
-    width_amount: int,
-) -> tuple[Literal[WHSettings.WEIGHT], int]: ...
+    width_amount: int | float,  # noqa: PYI041  # provide explicit for IDEs
+) -> tuple[Literal[WHSettings.WEIGHT], int | float]: ...
 
 
 @typing.overload
@@ -295,8 +297,13 @@ def simplify_width(
 
 def simplify_width(
     width_type: Literal["clip", "pack", "given", "relative", "weight"] | WHSettings,
-    width_amount: int | None,
-) -> Literal[WHSettings.CLIP, WHSettings.PACK] | int | tuple[Literal[WHSettings.RELATIVE, WHSettings.WEIGHT], int]:
+    width_amount: int | float | None,  # noqa: PYI041  # provide explicit for IDEs
+) -> (
+    Literal[WHSettings.CLIP, WHSettings.PACK]
+    | int
+    | tuple[Literal[WHSettings.RELATIVE], int]
+    | tuple[Literal[WHSettings.WEIGHT], int | float]
+):
     """
     Recombine (width_type, width_amount) into an width value.
     Inverse of normalize_width.
@@ -336,21 +343,23 @@ def normalize_height(
 
 @typing.overload
 def normalize_height(
-    height: tuple[Literal["weight", WHSettings.WEIGHT], int],
+    height: tuple[Literal["weight", WHSettings.WEIGHT], int | float],
     err: type[BaseException],
-) -> tuple[Literal[WHSettings.WEIGHT], int]: ...
+) -> tuple[Literal[WHSettings.WEIGHT], int | float]: ...
 
 
 def normalize_height(
     height: (
         int
         | Literal["flow", "pack", Sizing.FLOW, WHSettings.PACK]
-        | tuple[Literal["relative", "weight", WHSettings.RELATIVE, WHSettings.WEIGHT], int]
+        | tuple[Literal["relative", WHSettings.RELATIVE], int]
+        | tuple[Literal["weight", WHSettings.WEIGHT], int | float]
     ),
     err: type[BaseException],
 ) -> (
     tuple[Literal[Sizing.FLOW, WHSettings.PACK], None]
-    | tuple[Literal[WHSettings.RELATIVE, WHSettings.GIVEN, WHSettings.WEIGHT], int]
+    | tuple[Literal[WHSettings.RELATIVE, WHSettings.GIVEN], int]
+    | tuple[Literal[WHSettings.WEIGHT], int | float]
 ):
     """
     Split height into (height_type, height_amount).  Raise exception err
@@ -398,8 +407,8 @@ def simplify_height(
 @typing.overload
 def simplify_height(
     height_type: Literal["weight", WHSettings.WEIGHT],
-    height_amount: int | None,
-) -> tuple[Literal[WHSettings.WEIGHT], int]: ...
+    height_amount: int | float | None,  # noqa: PYI041  # provide explicit for IDEs
+) -> tuple[Literal[WHSettings.WEIGHT], int | float]: ...
 
 
 @typing.overload
@@ -422,8 +431,13 @@ def simplify_height(
         WHSettings.GIVEN,
         WHSettings.WEIGHT,
     ],
-    height_amount: int | None,
-) -> int | Literal[WHSettings.FLOW, WHSettings.PACK] | tuple[Literal[WHSettings.RELATIVE, WHSettings.WEIGHT], int]:
+    height_amount: int | float | None,  # noqa: PYI041  # provide explicit for IDEs
+) -> (
+    int
+    | Literal[WHSettings.FLOW, WHSettings.PACK]
+    | tuple[Literal[WHSettings.RELATIVE], int]
+    | tuple[Literal[WHSettings.WEIGHT], int | float]
+):
     """
     Recombine (height_type, height_amount) into a height value.
     Inverse of normalize_height.

diff --git a/urwid/widget/pile.py b/urwid/widget/pile.py
@@ -159,7 +159,8 @@ def __init__(
         widget_list: Iterable[
             Widget
             | tuple[Literal["pack", WHSettings.PACK] | int, Widget]
-            | tuple[Literal["weight", "given", WHSettings.WEIGHT, WHSettings.GIVEN], int, Widget]
+            | tuple[Literal["given", WHSettings.GIVEN], int, Widget]
+            | tuple[Literal["weight", WHSettings.WEIGHT], int | float, Widget]
         ],
         focus_item: Widget | int | None = None,
     ) -> None:
@@ -173,15 +174,13 @@ def __init__(
         *widget_list* may also contain tuples such as:
 
         (*given_height*, *widget*)
-            always treat *widget* as a box widget and give it *given_height* rows,
-            where given_height is an int
+            always treat *widget* as a box widget and give it *given_height* rows, where given_height is an int
         (``'pack'``, *widget*)
-            allow *widget* to calculate its own height by calling its :meth:`rows`
-            method, ie. treat it as a flow widget.
+            allow *widget* to calculate its own height by calling its :meth:`rows` method,
+            i.e. treat it as a flow widget.
         (``'weight'``, *weight*, *widget*)
             if the pile is treated as a box widget then treat widget as a box
-            widget with a height based on its relative weight value, otherwise
-            treat the same as (``'pack'``, *widget*).
+            widget with a height based on its relative weight value, otherwise treat the same as (``'pack'``, *widget*).
 
         Widgets not in a tuple are the same as (``'weight'``, ``1``, *widget*)`
 
@@ -192,7 +191,9 @@ def __init__(
         super().__init__()
         self._contents: MonitoredFocusList[
             Widget,
-            tuple[Literal[WHSettings.PACK], None] | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int],
+            tuple[Literal[WHSettings.PACK], None]
+            | tuple[Literal[WHSettings.GIVEN], int]
+            | tuple[Literal[WHSettings.WEIGHT], int | float],
         ] = MonitoredFocusList()
         self._contents.set_modified_callback(self._contents_modified)
         self._contents.set_focus_changed_callback(lambda f: self._invalidate())
@@ -237,7 +238,7 @@ def _validate_contents_modified(self, slc, new_items) -> None:
                 if any(
                     (
                         t not in {WHSettings.PACK, WHSettings.GIVEN, WHSettings.WEIGHT},
-                        (n is not None and (not isinstance(n, int) or n < 0)),
+                        (n is not None and (not isinstance(n, (int, float)) or n < 0)),
                     )
                 ):
                     invalid_items.append(item)
@@ -328,7 +329,9 @@ def contents(
         self,
     ) -> MonitoredFocusList[
         Widget,
-        tuple[Literal[WHSettings.PACK], None] | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int],
+        tuple[Literal[WHSettings.PACK], None]
+        | tuple[Literal[WHSettings.GIVEN], int]
+        | tuple[Literal[WHSettings.WEIGHT], int | float],
     ]:
         """
         The contents of this Pile as a list of (widget, options) tuples.
@@ -363,18 +366,21 @@ def contents(
         self,
         c: Sequence[
             Widget,
-            tuple[Literal[WHSettings.PACK], None] | tuple[Literal[WHSettings.GIVEN, WHSettings.WEIGHT], int],
+            tuple[Literal[WHSettings.PACK], None]
+            | tuple[Literal[WHSettings.GIVEN], int]
+            | tuple[Literal[WHSettings.WEIGHT], int | float],
         ],
     ) -> None:
         self._contents[:] = c
 
     @staticmethod
     def options(
         height_type: Literal["pack", "given", "weight"] | WHSettings = WHSettings.WEIGHT,
-        height_amount: int | None = 1,
+        height_amount: int | float | None = 1,  # noqa: PYI041  # provide explicit for IDEs
     ) -> (
         tuple[Literal[WHSettings.PACK], None]
-        | tuple[Literal["given", "weight", WHSettings.GIVEN, WHSettings.WEIGHT], int]
+        | tuple[Literal[WHSettings.GIVEN], int]
+        | tuple[Literal[WHSettings.WEIGHT], int | float]
     ):
         """
         Return a new options tuple for use in a Pile's :attr:`contents` list.
@@ -386,9 +392,9 @@ def options(
 
         if height_type == WHSettings.PACK:
             return (WHSettings.PACK, None)
-        if height_type not in {WHSettings.GIVEN, WHSettings.WEIGHT}:
-            raise PileError(f"invalid height_type: {height_type!r}")
-        return (height_type, height_amount)
+        if height_type in {WHSettings.GIVEN, WHSettings.WEIGHT} and height_amount is not None:
+            return (height_type, height_amount)
+        raise PileError(f"invalid combination: height_type={height_type!r}, height_amount={height_amount!r}")
 
     @property
     def focus(self) -> Widget | None: