De-wx'ing

[Utumno]

All wx code must eventually end up in balt gui. This does not simply mean that one must create wrappers for wx classes/constants and drop them into gui - it means that gui must export an API - so gui should not return wx items on which wx methods will be called - must wrap those methods into class methods it exports - in other words we must not only eliminate wx imports but also wx calls on wx objects returned by gui functions (such as Bind). This (apart its obvious, I hope, value vis a vis code quality) will also make it easier to move to later versions of wxPython / offer an entirely different backend. There is already a great deal of it done (edited out of this issue). But there is also an awful lot left to be done. So:

• sizers - that's the biggest one. According to what I wrote above, defining say wxEXPAND = wx.EXPAND in gui is not the way to go. Rather the usage patterns of sizers in Bash must be explored from someone who groks the wx sizers API and an API be crafted in gui which defines what one wants (as opposed to how to achieve it).
  • wx.LEFT etc, 0 does make any cense ?
  • I would like to go through the uses of sizers a bit and see what is left out - the wx.TOP etc stuff for instance - I have a plan for adding them in the LayoutOptions
  • rename default_border etc to item_border etc - this is confusing (and much too verbose)
• an Images API is also badly needed - I hacked up a couple classes but that's exactly that - a hack - see 36e7f89. -> new issue Rework internal image API to account for change of size icons (and general maintainability) #366
• rest of functions in balt to classes (like checkBox, v/hsizer etc)
• A single package with wx imports. Single and as small as possible -> gui. Files that currently import wx (and they shouldn't) are:
  • basher/__init__.py: 7 usages left (mostly wx.Notebook but also wx.EVT_CONTEXT_MENU, wx.NOT_FOUND, wx.Platform, wx.EVT_NOTEBOOK_PAGE_CHANGED)
  • constants.py: nasty, our settings defaults depended on wx constants
  • dialogs.py: sizers, wx.TE_RICH2, wx.ColourPickerCtrl, wx.Icon
  • frames.py:
    • this needs attention. wx.lib.iewin --> this must go! Also a lot of classes in need of wrapping
    • wx.ArtProvider_GetBitmap (icons API !)
    • wx.FlexGridSizer
    • One random usage of wx.NOT_FOUND
  • gui_fomod.py: same problem as belt below
  • gui_patchers.py: sizers, wx.Window
  • patcher_dialog.py: sizers, event binding
  • settings_links.py: sizers
    And outside basher:
  • belt.py: the wizard dialog
  • id parameters from balt functions/constructors must be dropped
  • balt functions such as button must become classes and be inherited by OkButton, CancelButton etc

Some newer TODOs:

• where did the tracebacks go? -> fixed in 6f121cb
• the issue with warnings on booting still allowing the user to interact with main frame -> fixed on wx4 yey! -> https://trac.wxwidgets.org/ticket/17816
• the progress dialog - ugh. Our whole CancelError stuff is not working anymore (and I never liked that exception control flow anyway). More importantly, there is the strange issues of losing focus and I think it's the progress window again. -> wx bug, fixed by switching to GenericProgressDialog
• related to the above - make sure we don't refresh twice - we must follow a bit the events (with good old prints) and make sure they are processed once - I am thinking also of cases we display dialogs and such, is RefreshData triggered? More @balt.conversation ? We could maybe add an _is_shown attribute and use the evt_activate argument to only refresh when the _is_shown becomes True from False -> seems working ok but keep an eye open!
• not sure about properties yet - sometimes it's ok but sometimes methods are preferred - more explicit - WIP but stabilizing - current code is fine
• events - it seems wx did not doubly register a handler - see notes on 4766e10. We do and we skip by default which means we have to go through the previous uses and carefully evaluate if skip is needed. Seems it's not the default because it is not meant to be used often. So there are system handlers that might need to be skipped - could not really understand their docs: https://wxpython.org/Phoenix/docs/html/events_overview.html#how-events-are-processed - I guess system handlers run after user ones
  • revisit event handling during boot - especially size events need be handled after setting sizes before maximizing apparently etc - see 4d70274 (Attempt to fix installers tab positions not being set)
• balt will no longer serve a purpose once we're done - all balt code should either go into basher (highly specific Wrye Bash code) or into gui (pure abstractions over wx). Useful rule of thumb: if you think the class / method could be easily copied into another project without dragging core Bash code along while still being useful, it's gui
• revisit locale handling - see> wxWidgets/Phoenix#1702 from wxPython 3 Upgrade #15 (comment)

Under #163

[Utumno]

I am working on this and will hopefully close it for 312, as it's been around for a while :P and keeps blocking some nice refactoring - some of it was ideas that I had for a while - encapsulate the JIT behavior of some of the balt/gui classes is the Lazy class - wrap Objects above Window another one. Now this makes controlling access to the _native_widget more imperative, think also of Bash ctd'ing very very hard when for instance wx.App is not initialized (or...?). Think also of switching gui backend, even if we won't ever I hope, a gui wrapper would profit from being designed with this in mind. I was thinking of something like:

+def _native(*, native_get: str = None, native_set: str = None):
+    """Decorator to delegate method calls to the native widget. Used to
+    control native widgets access (and potentially swap gui backend)."""
+    if not (bool(native_set) ^ bool(native_get)): # xor them!
+        raise ValueError(f'Pass either {native_get=} or {native_set=}')
+    def call_native(meth):
+        if native_get:
+            @functools.wraps(meth)
+            def _call_native(self, *args, **kwargs):
+                return getattr(self._native_widget, native_get)(*args, **kwargs)
+        if native_set:
+            @functools.wraps(meth)
+            def _call_native(self, *args, **kwargs):
+                getattr(self._native_widget, native_set)(*args, **kwargs)
+        return _call_native
+    return call_native
+
 class _AComponent:
@@ -207,2 +224,3 @@ def set_component_name(self, new_ctrl_name: str):
     @property
+    @_native(native_get='IsShown')
     def visible(self) -> bool:
@@ -212,5 +230,5 @@ def visible(self) -> bool:
         :return: True if this component is currently visible."""
-        return self._native_widget.IsShown()
 
     @visible.setter
+    @_native(native_set='Show')
     def visible(self, is_visible: bool):
@@ -219,3 +237,2 @@ def visible(self, is_visible: bool):
         :param is_visible: Whether or not to show this component."""
-        self._native_widget.Show(is_visible)
 

Has the advantage of pruning and singling out most boilerplate uses of _native_widget, can be used to control access in a central place and can be conditionally passed different functions for different backends (I am not familiar with any so this may not apply but I guess we should have a _native_widgtet too and access similar methods - if not pass different functions). One default is harder typing - in particular we cant control click on the gui method and jump to the stub but this is still a couple clicks away (plus we should not be revisiting much the code in gui once frozen).

[lojack5]

I've been slowly working on a similar (more for a generic solution) attack on this same idea. I like the approach. My pet project is taking a lot of work because I'm really trying to get the IDE/static type-checkers to still be able to do some helpful work for me. I want the typechecker to be able to point out when the method I'm forwarding to actually does have the same signature of the one I'm wrapping (just passing in the name of the method basically removes all ability of IDEs/etc to help you out there). It's a hard problem getting that working properly, especially with all the different types of functions that could be accessed on an instance (bound method, unbound method, class method, static method, attribute that's a function but not an instance method, properties). Anyway, it's a fun little problem to work on fleshing out all those details, who know's if I'll ever get it to a good working state though.

That aside, this is good enough for it's purposes :)

[Utumno]

An interesting typing exercise (passed when minimal to no code is added :P) would be to use typing for the events handling in our gui library - useful judging from the couple wrong params I passed to the on_drag callbacks recently.

[lojack5]

Not too hard to do, but would require a least one type definition (per event handler signature) when first defining the event handler. It can be done in a way to even work with lambdas properly (which are notoriously hard to get to work with type information). I'll mock something up, but they do end up using TypeVar and TypeVarTuple, which means we should hold off till 3.12 to make it much cleaner PEP 695 is a godsend for cleaning up most of the boilerplate cruft involved.

[lojack5]

Ok, lojack-typed-eventhandler is an example of what can be done (requires python 3.12, hence the CI failure). There's one thing blocking this really being useful, and one edge case that slightly hinders usability:

Blocker - wxPython type stubs don't have return type information. Since most of our argument processors are calling wx methods on event objects, the typechecker/IDE can't properly infer the types for the event handler.
Minor niggle - lambda functions defined inline (inside the call to _AComponent._evt_handler) work well, because they can get type information from _evt_handler itself. Lambda functions defined elsewhere have almost no useable type information, because the event parameter has no type information in that context.

I've already wanted to revisit the wxPython typestubs (they're autogenerated with a custom python script), this might make me go back and actually work on it.

Here's an example of what VSCode can infer about the return types - it can't figure out what GetLabel is returning, due to those type-stubs not having the information.

[Utumno]

Thanks Lo! Seems like typing is going beta :P Will have to have a closer look (need to set up 3.12 and I fell into a rabbit hole of regexes - hopefully done soon)

[lojack5]

If you just want to play around on 3.12, everything seems to work fine out of the box, we just can't package (pygit2 not available) and PDFs aren't shown (pymupdf not available).