Python type-stubs problems

[timrid]

Operating system: Windows 11
wxPython version & source: wxPython-4.2.3a1.dev5812+c3092be4-cp311-cp311-win_amd64
Python version & source: Python 3.11.6

Description of the problem:
Thanks a lot @lojack5 for creating #2468 and @swt2c for merging it. I tried the above mentioned snapshot in my codebase and found the following problems with VSCode and pylance v2024.12.1:

1. In wx.Frame, wx.Dialog and every class that inherits from it, the parameter parent can also be None and not only wx.Window.
   

2. Properties are not correctly typed. Eg. wx.Size().GetWidth() is int, but wx.Size().Width is of type Any.
   

3. wx.TIMER_CONTINUOUS & wx.TIMER_ONE_SHOT should be bool instead of int, because in Timer.Start(...) the parameter oneShot is also bool
   

4. wx.TreeItemId.__bool__() has to return bool instead of int
   

5. wx.dataview.DataViewItem.__bool__() has to return bool instead of int
   

6. wx.DateTime can be constructed from datetime.datetime and datetime.time, so an __init__ overload should be available for these types.
   

[lojack5]

Hi! Yeah, the system used for generating type-stubs isn't perfect at all. Most of the info is first grabbed from the wxWidgets source code, then transformed via sip as a wrapper into Python. The type information is almost entirely generated from the C++ signatures. So I'll go through these, but not as an excuse but more a documenting why it currently happens, and asking for if there might be a way to get this information on to the etg scripts that can then use the information:

1. None for parent: In Python, None and an actual object are different types. In C++ it's just the difference between a NULL pointer and a non-NULL pointer (same types). I don't know how to pass the info along that a NULL pointer is fine in this situation. Someone more familiar with the beginning of the toolchain might? For reference, this is the C++ signature that eventually generates the type-stub for this:

wxWindowMSW(wxWindow *parent,
          wxWindowID id,
          const wxPoint& pos = wxDefaultPosition,
          const wxSize& size = wxDefaultSize,
          long style = 0,
          const wxString& name = wxASCII_STR(wxPanelNameStr))

2. This seems to be a limitation of pyright/pylance, where using property as a decorator grabs the information, but using it as a callable does not. Here's an example of what I mean:

class Test:
  @property
  def a(self) -> int: ...
  @a.setter
  def a(self, value: int) -> None: ...


  def _get_b(self) -> int: ...
  def _set_b(self, value: int) -> None: ...

  b = property(_get_b, _set_b)



t = Test()
t.a # inferred as int
t.b # inferred as Any

3. This one is probably fixable, though I'm not sure how at the moment. The wxWidgets code uses a #define for this constant as false. Original pystub generator just assigned all #defines as = 0, so the new version detects that as an int and hints it that way. There's might be a way to still see the false at that point in the code and correctly assign it a Literal[False] instead (or even bool.
4. This one I'm not sure of at all, because I'm not sure where in sip the __bool__ method is generated. Help here would be nice.
5. Same as 4
6. This one I would also need help on. The C++ code obviously doesn't have an overload for the Python datetime.datetime object. There's probably a handwritten overload added in somewhere in the chain (or maybe not even at all, since the originally generated signatures was just (*args, **kwargs)).

Anyway, that's the current state, and I'm only familiar with the part of the code that takes what etg passes off to the typestub script. I'm guessing 3, 4, 5 are probably fixable relatively easily. 6 Might be fixable with a custom overload added in the sip wrapper?

[swt2c]

I think I know how to fix 4&5, plus possibly 3, so I can take care of those.

[swt2c]

I think I know how to fix 4&5, plus possibly 3, so I can take care of those.

OK, I thought I had an idea of how to fix 4&5, but it didn't work. The issue is because these __bool__ slots are defined as C++ methods like this:

c.addCppMethod('int', '__bool__', '()', "return self->IsOk();")

My initial thought was that we could change these to bool and that would solve the problem. However, we can't do that because the Python/C interface requires them to be int. So we'll need to find somewhere else to fix it.

[lojack5]

Hmm, I don't know enough about the process there to know the best way to fix that.

I was thinking about 3 some more, it will take a little work (meaning, make sure the property generating part has access to the return types of the getter), but I could probably make it generate properties like this instead:

Before:

    ...
    Width = property(GetWidth, SetWidth)
    ...

After:

    ...
    @property
    def Width(self) -> int: ...
    @Width.setter
    def Width(self, width: int, /) -> None: ...
    ...

With appropriate checks for whether there are setters/getters (a few only have a setter, a few only have a getter).

The only exception here is for setter-only properties it'll still have to be:

    ...
    setter_only_property = property(None, SetterMethod)
    ...

At least this way most of the properties exposed will be type-inferred correctly. Though there's still the case where wxPython accepts some types that automatically get converted to the right type, as an example, these are all valid:

window.SetBackgroundColour('black')
window.SetBackgroundColour((0, 0, 0))
window.SetBackgroundColour(wx.Colour(0, 0, 0))

But the generated type-stub only has the wx.Colour variant.

[timrid]

To point 1:
According to the wxWidgets Book only Top-Level Windows support a None parent.

Top-Level Windows
Windows are broadly divided into top-level windows (wxFrame, wxDialog,
wxPopup) and other windows. Only top-level windows may be created with a
NULL parent, and only top-level windows have delayed destruction (they are
not deleted until idle time, when all other events have been processed). Except
for pop-up windows, top-level windows can have decorations such as title bars
and close buttons and can normally be dragged around the screen and resized,
if the application allows it.

Maybe it is somehow possible to build in a detection whether the current class inherits from wx.TopLevelWindow and set the parent parameter to wx.Window | None for all these classes. This is just an idea though, I'm not familiar enough with the whole code generation of wxPython to be able to tell if this is possible.

However, this point bothers me the most of all...

Currently you can only use the following workaround to avoid getting an error message (or you can use # type: ignore which is even worse style).

import typing
import wx

NONE_WINDOW = typing.cast(wx.Window, None)

wx.Frame(NONE_WINDOW)

To point 2:
The problem with 2 is, that pylance uses the type hints for property from typshed which uses only Any types. If we modify it slightly we can make our own _wxProperty type for the wxPython type stubs. This is maybe easier to implement than to use your suggestion @lojack5 .

import typing

Tgetter = typing.TypeVar("Tgetter")
Tsetter = typing.TypeVar("Tsetter")

class _wxProperty(property, typing.Generic[Tsetter, Tgetter]):
    fget: typing.Callable[[Any], Tgetter] | None
    fset: typing.Callable[[Any, Tsetter], None] | None
    def __init__(
        self,
        fget: typing.Callable[[Any], Tgetter] | None = None,
        fset: typing.Callable[[Any, Tsetter], None] | None = None,
    ) -> None: ...
    def __get__(self, instance: Any, owner: type | None = None, /) -> Tgetter: ...
    def __set__(self, instance: Any, value: Tsetter, /) -> None: ...

[lojack5]

1. I might be able to get base class detection working by testing in here and here, though while the generator does have base class information, I'm pretty sure it's only the string names of the direct base classes. Would take some work to get it working correctly to detect a child of TopLevelWindow - and might not even work right anyway without some tweaks to the extractor code (which I have zero experience with). EDIT: yep, the extractors only have base class names. If it happens to have all base classes and not just direct base classes, it'll be easy. It also has subclass info, which could potentially be a better place to grab info from. Though then it's still a process of some tricky edits to do this translation only on __init__, only on arguments named parent, and only if it's typed as Window. Lot's of special casing things. Anyway, I'll have to attempt things to see if it's even possible without really nasty code.

2. Personally I'd rather just rewrite to write out properties using the decorator syntax, since I know that works in pylance, rather than introducing a typing only class for hinting, but 🤷

3. Can probably fix this one by adding another check here for false and true.

I'll have to try to setup a build environment again, when I tried recently I was having issues with doxygen detection (building on windows is sort of a "figure it out" kind of situation).

[echoix]

I remember reading that typing properties was still a bit harder currently.

When improving the typing annotations, I would like to suggest to compare to the typing files that sip can generate by enabling pep484-pyi = true (instead of setting it false) in build.py

[tool.sip.bindings.{base}]
docstrings = true
release-gil = true
exceptions = false
tracing = {tracing}
protected-is-public = false
generate-extracts = [\'{extracts}\']
pep484-pyi = true

They are really clean and detailed, and as .pyi shouldn't contain docstrings (* see note), they don't.

One difference between these sip-generated ones and the ones manually made up here is that the sip-generated ones are for the extension modules that sip generates. So they are exact, but only for these extension modules. Notice the underscores: they match the .so files (on linux). The non underscore .pyi files, like core.ypi, adv.pyi, aui.pyi, are .pyi files matching the additionnal .py files that wxPython creates to wrap these "private" extension modules. So sip couldn't generate the .pyi files for the wrapper of the sip-generated wrapper.
One other difference that wxPython does, is that it makes nicer names for constants. But on that topic, I think that the sip-generated .pyi files have an incredibly useful representation of all constants/enums/flags: As a class, and typed, so it remains an int if needed, but can be readily used by type checkers, like mypy, pyright, pyre, etc..

My exploration of last weekend isn't finished, but I was trying to get the best of both worlds by getting the wxPython PI generator to output the classes as subclasses of the types in the extension modules (the ones with underscores). Overloads with correct types appear (from the sip-generated files), and docstrings (from wxPython PI generator) are kept, at least for vscode. I tried out at first by importing the sip-generated .pyi files into my container's site-packages, then manually editing the files to see the result.
The next part was to actually have these new files copied to the sdist on build, and I managed that. One of my working branch's PR is echoix#3 (if you want to download sdist artifacts)
I think a useful point is to have a file named "py.typed" (they call it a marker) so mypy can accept it, and having its content being "partial" with an "\n" (newline) after, it makes type checkers know that it needs to continue searching when a module is not found, see PEP 561

(*): .pyi shouldn't contain docstrings, generally... Though in the case here, since the implementation part of the code isn't really python, and the extension modules don't have their docstrings generated there (I assume, as clearing the .pyi of some classes didn't allow my IDE to find them from the classes), the docstrings here are the only way I see for now to have some docs, and it's perfectly fine.

[lojack5]

I have 3) fixed on a local copy, digging in to see if it's feasible to fix 1) currently (it's...complicated).

@echoix If you have experience making sip generate the type-stubs, I'd be more than happy to see that personally (can't speak for the maintainers). I'd assume it'd have better accuracy and correctness than what the current homebrew(?) script generates.

[echoix]

I have 3) fixed on a local copy, digging in to see if it's feasible to fix 1) currently (it's...complicated).

@echoix If you have experience making sip generate the type-stubs, I'd be more than happy to see that personally (can't speak for the maintainers). I'd assume it'd have better accuracy and correctness than what the current homebrew(?) script generates.

My experience with sip specifically starts only since saturday, but you can take a look at these by downloading the deist artifacts of the runs here: echoix#3

They are accurate (until proven otherwise) for the compiled extension modules. For example, extra methods implemented in Python attached to it after loading aren't included, as sip wasn't involved there

[lojack5]

Regarding None for parent: I've done some work and have something that appears to work, here are the affected signatures:

TopLevelWindow.__init__ (self, parent: Optional[Window], id: int=ID_ANY, title: str='', pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_FRAME_STYLE, name: str=FrameNameStr) -> None
Dialog.__init__ (self, parent: Optional[Window], id: int=ID_ANY, title: str='', pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_DIALOG_STYLE, name: str=DialogNameStr) -> None
DirDialog.__init__ (self, parent: Optional[Window], message: str=DirSelectorPromptStr, defaultPath: str='', style: int=DD_DEFAULT_STYLE, pos: Point=DefaultPosition, size: Size=DefaultSize, name: str=DirDialogNameStr) -> None
FileDialog.__init__ (self, parent: Optional[Window], message: str=FileSelectorPromptStr, defaultDir: str='', defaultFile: str='', wildcard: str=FileSelectorDefaultWildcardStr, style: int=FD_DEFAULT_STYLE, pos: Point=DefaultPosition, size: Size=DefaultSize, name: str=FileDialogNameStr) -> None
Frame.__init__ (self, parent: Optional[Window], id: int=ID_ANY, title: str='', pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_FRAME_STYLE, name: str=FrameNameStr) -> None
MessageDialog.__init__ (self, parent: Optional[Window], message: str, caption: str=MessageBoxCaptionStr, style: int=OK|CENTRE, pos: Point=DefaultPosition) -> None
GenericMessageDialog.__init__ (self, parent: Optional[Window], message: str, caption: str=MessageBoxCaptionStr, style: int=OK|CENTRE, pos: Point=DefaultPosition) -> None
ColourDialog.__init__ (self, parent: Optional[Window], data: Optional[ColourData]=None) -> None
MultiChoiceDialog.__init__ (self, parent: Optional[Window], message: str, caption: str, choices: List[str], style: int=CHOICEDLG_STYLE, pos: Point=DefaultPosition) -> None
MultiChoiceDialog.__init__ (self, parent: Optional[Window], message: str, caption: str, n: int, choices: str, style: int=CHOICEDLG_STYLE, pos: Point=DefaultPosition) -> None
SingleChoiceDialog.__init__ (self, parent: Optional[Window], message: str, caption: str, choices: List[str], style: int=CHOICEDLG_STYLE, pos: Point=DefaultPosition) -> None
FindReplaceDialog.__init__ (self, parent: Optional[Window], data: FindReplaceData, title: str='', style: int=0) -> None
MDIParentFrame.__init__ (self, parent: Optional[Window], id: int=ID_ANY, title: str='', pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_FRAME_STYLE|VSCROLL|HSCROLL, name: str=FrameNameStr) -> None
MDIChildFrame.__init__ (self, parent: Optional[MDIParentFrame], id: int=ID_ANY, title: str='', pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_FRAME_STYLE, name: str=FrameNameStr) -> None
FontDialog.__init__ (self, parent: Optional[Window]) -> None
FontDialog.__init__ (self, parent: Optional[Window], data: FontData) -> None
RearrangeDialog.__init__ (self, parent: Optional[Window], message: str, title: str='', order: List[int]=[], items: List[str]=[], pos: Point=DefaultPosition, name: str=RearrangeDialogNameStr) -> None
MiniFrame.__init__ (self, parent: Optional[Window], id: int=ID_ANY, title: str='', pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=CAPTION|RESIZE_BORDER, name: str=FrameNameStr) -> None
TextEntryDialog.__init__ (self, parent: Optional[Window], message: str, caption: str=GetTextFromUserPromptStr, value: str='', style: int=TextEntryDialogStyle, pos: Point=DefaultPosition) -> None
PasswordEntryDialog.__init__ (self, parent: Optional[Window], message: str, caption: str=GetPasswordFromUserPromptStr, defaultValue: str='', style: int=TextEntryDialogStyle, pos: Point=DefaultPosition) -> None
NumberEntryDialog.__init__ (self, parent: Optional[Window], message: str, prompt: str, caption: str, value: int, min: int, max: int, pos: Point=DefaultPosition) -> None
PreviewFrame.__init__ (self, preview: PrintPreview, parent: Optional[Window], title: str="PrintPreview", pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_FRAME_STYLE, name: str=FrameNameStr) -> None
PrintAbortDialog.__init__ (self, parent: Optional[Window], documentTitle: str, pos: Point=DefaultPosition, size: Size=DefaultSize, style: int=DEFAULT_DIALOG_STYLE, name: str="dialog") -> None

Are there any false positives here? Any missing methods that should be detected as needing the Optional treatment? I've never used half of these to know if it is actually OK to pass in a None parent (MDIChildFrame seems suspicious).

Detection is done by finding subclasses of TopLevelWindow with parent as one of their arguments. Note: some other classes satisfy this, but already had parent=None in their argstring, so they already currently are marked as Optional. These are just the ones with non- defaulted to None parent parameters.

[Metallicow]

I would like you others to take a look... Maybe just a small one at lojacks code/request. This would be important to a certain extent.