Split widget and introduce base enums #595

penguinolog · 2023-08-17T07:11:15Z

Split base widget (widget module), container widgets (container module) and decoration widgets (decoration and graphics modules) into several modules under subpackage widget.

Introduce string-compatible enums for sizing, align, valign, wrap mode and base width-height settings.
Enforce automating imports sorting and code formatting + static validation for introduced changes.

Checklist

I've ensured that similar functionality has not already been implemented
I've ensured that similar functionality has not earlier been proposed and declined
I've branched off the master or python-dual-support branch
I've merged fresh upstream into my branch recently
I've ran tox successfully in local environment

* Backward compatible (subclass `str`) * Auto-format and static check for widget

* Backward compatible (subclass `str`)

* Use enums * Enforce code style and static checking

* Use backward compatible exports in package

* import to old modules for backward compatibility * split constants from widget module and extend WHSettings (CLIP = WrapMode.CLIP, FLOW = Sizing.FLOW) * proper normalize_*, simplify_* (check for None, partial handle `RELATIVE`)

* use backward compatibility imports in module

* use backward compatibility imports in module * leave python logo widget untouched

coveralls · 2023-08-17T07:12:43Z

coverage: 73.544% (+0.09%) from 73.458% when pulling 9e1a128 on penguinolog:split_widget_enums into a8c0d71 on urwid:master.

penguinolog · 2023-08-17T07:13:23Z

urwid/widget/constants.py

technically after enums introduction and move of sizing helpers it's not constants, but I have no idea about better naming

penguinolog · 2023-08-17T07:14:53Z

urwid/widget/wimp.py

I was lazy and not splitted this module: it's < 1000 LOC's and can be splitted later if will be too complex

penguinolog · 2023-08-17T07:15:51Z

urwid/listbox.py

automatically re-formatted after imports change (no manual work)

penguinolog · 2023-08-17T07:20:35Z

urwid/tests/test_signals.py

@@ -25,6 +26,7 @@ def test_connect(self):
        edit.set_edit_text('another text')
        handler.assert_not_called()

+    @unittest.skipIf(sys.implementation.name == "pypy", "WeakRef works differently on PyPy")


this test is expecially unstable on local PyPy run.
Also manual experiments shown, that everything garbage collector related work can work differently on PyPy

penguinolog · 2023-08-17T07:21:06Z

urwid/tests/test_doctests.py

-        urwid.widget,
-        urwid.wimp,
-        urwid.decoration,
+        urwid.widget.attr_map,


doctests need explicit paths :(

penguinolog · 2023-08-17T07:50:50Z

Validated compatibility against downstream packages: khal and zulip-terminal

penguinolog · 2023-08-17T07:52:01Z

urwid/widget/__init__.py

+    VAlign,
+    WHSettings,
+    WrapMode,
+    normalize_align,


normalize_* / simplify_* are useful in downstream code, but was never exported "public"

penguinolog · 2023-08-17T07:53:00Z

urwid/widget/__init__.py

+from .widget_decoration import WidgetDecoration, WidgetDisable, WidgetPlaceholder
+from .wimp import Button, CheckBox, CheckBoxError, RadioButton, SelectableIcon
+
+__all__ = (


__all__ needed due to backwards-compatibility constants definition (as it's not used locally, it's defined as "public" explicit)

JC3 · 2023-09-21T21:48:28Z

"clip" no longer works for the width parameter to Padding because of this commit (specifically, because of the conversion to enums).

  File "/Users/cipriani_pro2023/pyenv-brixel-newprotobuf/lib/python3.9/site-packages/urwid/widget/padding.py", line 137, in __init__
    self._width_type, self._width_amount = normalize_width(width, PaddingError)
  File "/Users/cipriani_pro2023/pyenv-brixel-newprotobuf/lib/python3.9/site-packages/urwid/widget/constants.py", line 269, in normalize_width
    raise err(
urwid.widget.padding.PaddingError: width value 'clip' is not one offixed number of columns, 'pack', ('relative', percentage of total width), 'clip'

The root cause is that WHSettings.CLIP is assigned a value of WrapMode.CLIP, which causes the in statement at https://github.com/penguinolog/urwid/blob/9e1a128bf80805ee84d8cb1af5b5b5017817240d/urwid/widget/constants.py#L259 to always evaluate to False for "clip".

Note that the following example program will print False:

import enum

class OtherTest(str, enum.Enum):
    CLIP = "clip"
    
class Test(str, enum.Enum):
    PACK = "pack"
    CLIP = OtherTest.CLIP
    
print("clip" in (Test.PACK, Test.CLIP))

The fix is to assign WHSettings.CLIP to the actual string value "clip". The same should probably also be done for WHSettings.FLOW.

The consequence at the moment is that release 2.2.0 can't be used in programs that use "clip" for Padding.

ethanfurman · 2023-09-22T13:17:14Z

Using the StrEnum in Python 3.12 does function as expected. [Edit: but don't do it this way -- see below.]

penguinolog · 2023-09-22T13:30:13Z

On python 3.10 synthetic tests it also worked, but release is for python 3.7+ - need to fix now and wait for old python releases EOL.

ethanfurman · 2023-09-22T13:37:38Z

Further explanation: When a type is mixed in to an enum, that type is used to generate the .value, so the value of Test.CLIP was generated by str(OtherTest.CLIP) (which is "OtherTest.CLIP"). In Python 3.12 StrEnum was added, and IntEnum and IntFlag modified, so that str(enum_member) was the same as str(enum_member.value) precisely to avoid the problem of constants converted to enums no longer working correctly you are seeing now.

Having said that, using one enum member as the value of another enum member is not intended, and can easily cause confusion:

import enum

class OtherTest(enum.StrEnum):
    CLIP = "clip"

class Test(enum.StrEnum):
    PACK = "pack"
    CLIP = OtherTest.CLIP

"clip" in (Test.PACK, Test.CLIP))
# True
Test.CLIP == OtherTest.CLIP
# True
Test.CLIP is OtherTest.CLIP  # given the above definition, one could expect this to be True
# False

Widget refactoring: Switch to enum usage for attributes

f1835d4

* Backward compatible (subclass `str`) * Auto-format and static check for widget

penguinolog added the refactoring label Aug 17, 2023

penguinolog requested a review from ulidtko August 17, 2023 07:11

Aleksei Stepanov added 9 commits August 17, 2023 09:11

Widget refactoring: Switch to enum usage for command_map

3a33c09

* Backward compatible (subclass `str`)

Widget refactoring: update wimp

5aeecce

* Use enums * Enforce code style and static checking

Move and split widget module

853a133

* Use backward compatible exports in package

Move and split decoration & container

0d3586d

* import to old modules for backward compatibility * split constants from widget module and extend WHSettings (CLIP = WrapMode.CLIP, FLOW = Sizing.FLOW) * proper normalize_*, simplify_* (check for None, partial handle `RELATIVE`)

Move and split wimp: save wimp part and separate popup

91da845

* use backward compatibility imports in module

Partial move of graphix

7983a2d

* use backward compatibility imports in module * leave python logo widget untouched

Expose enums

c300d17

Update imports and auto-format listbox module

eab509a

update imports in vterm

838b4e6

penguinolog force-pushed the split_widget_enums branch from 87f03e9 to 838b4e6 Compare August 17, 2023 07:11

Fix type annotations for widgets

9e1a128

penguinolog commented Aug 17, 2023

View reviewed changes

penguinolog requested review from wardi and removed request for ulidtko August 17, 2023 11:13

penguinolog merged commit c4fd30f into urwid:master Sep 4, 2023

penguinolog deleted the split_widget_enums branch September 4, 2023 07:26

JC3 mentioned this pull request Sep 21, 2023

"clip" no longer works for Padding width in 2.2.0 #606

Closed

ethanfurman mentioned this pull request Sep 22, 2023

Fix: deep TextEnum was improperly resolved #609

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split widget and introduce base enums #595

Split widget and introduce base enums #595

penguinolog commented Aug 17, 2023 •

edited

Loading

coveralls commented Aug 17, 2023 •

edited

Loading

penguinolog Aug 17, 2023

penguinolog Aug 17, 2023

penguinolog Aug 17, 2023

penguinolog Aug 17, 2023

penguinolog Aug 17, 2023

penguinolog commented Aug 17, 2023

penguinolog Aug 17, 2023

penguinolog Aug 17, 2023

JC3 commented Sep 21, 2023 •

edited

Loading

ethanfurman commented Sep 22, 2023 •

edited

Loading

penguinolog commented Sep 22, 2023

ethanfurman commented Sep 22, 2023

Split widget and introduce base enums #595

Split widget and introduce base enums #595

Conversation

penguinolog commented Aug 17, 2023 • edited Loading

Checklist

coveralls commented Aug 17, 2023 • edited Loading

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

penguinolog commented Aug 17, 2023

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

penguinolog Aug 17, 2023

Choose a reason for hiding this comment

JC3 commented Sep 21, 2023 • edited Loading

ethanfurman commented Sep 22, 2023 • edited Loading

penguinolog commented Sep 22, 2023

ethanfurman commented Sep 22, 2023

penguinolog commented Aug 17, 2023 •

edited

Loading

coveralls commented Aug 17, 2023 •

edited

Loading

JC3 commented Sep 21, 2023 •

edited

Loading

ethanfurman commented Sep 22, 2023 •

edited

Loading