Improve Python type-stubs #2468
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The line-wrapping causes issues once the python signatures become too long, as textwrap isn't smart enough to split the lines on valid continuation points for code. I had one instance of splitting a line in the middle of a string -> SyntaxError on next run of etg due to the generated PYI file having an unterminated string. Specificially, disable splitting for lines that start (ignoring spaces) with a specific string - in this case any line starting with the name of the function or method this is a docstring for.
This allows for building `FixWxPrefix.cleanType` on top of it, for use in processing type-hint strings in the future. It also exposes the method to `FunctionDef.makePyArgString` in the future, which has easier access to the types of arguments and returns. And possibly further in the future, other `***Def` classes can make use of it (constant definitions, etc).
Leverages the `writeSection` machinery, with a tweak to specify to add a new section to the beginning of a file, after the header. This ensures the required imports gets updated (and also only imported once per file) if new imports are needed for type-hints. Hint: there's a few more to come.
One unexpected type of '...' required adding a new transformation that modifies both the name and the type to just '*args', so added a preferred method `FixWxPrefix.parseNameAndType` which processes both strings at once. Also fixes cleanType to recursively call cleanType on sub-types (was improperly calling cleanName). With this, method and function signatures now have type annotations which are mostly correct (100% correct in the "it compiles" sense). Thankfully, the incorrect type-hints don't cause errors due to using stringized annotations (by importing annotations from __future__). Importantly, the overload signatures now have been fully sanitized. Before this, there was one instance of a variable named `is`, and another named `/Transfer/` - both invalid identifiers. I stopped looking after those. Since theses signatures are valid Python code, this opens up the opportunity to use `typing.overload` to fully expose those. Edge-cases in type-hints will be addressed in later commits.
These will be changing to annotation statements, so FixWxPrefix needs to be able to detect this still (proper thing to look for in this case is `ast.AnnAssign`).
With the work from the previous commits, it's as simple as no longer lopping off the args string at the '->' marker. (And one minor fixup to the makePyArgsString code).
By directly referencing their setter and getter methods, and due to the typing work already done for methods, we now have type information attached to properties. There are a few edge cases of setters/getters not having the proper number of arguments for a getter(0) or setter(1), but most cases are handled. The incorrect number of arguments may be missing default arguments from what the extraction code figures out from the C++ code?
Use the more generic type rather than a literal type. Before, a type-checker would infer an int defined this way as `Literal[0]` vs the more correctly generic `int` for example.
Named enums with 'Flags' in the name use `enum.IntFlag`, all other enums use `enum.IntEnum`. We have to bring the enum members into the containing scope to match the original behaviour. Further, since these enums are typing information only, and not actually in wx proper, we prevent them from appearing to leak the the user by marking as non-exported (prepend '_' to the name), then make a TypeAlias to the enum or an int. This way type signatures still claim to accept integers as appropriate. I like this solution best, because we preserved the information about which enum members are expected for method/function parameters, while not leaking non-existant classes out of the type-stubs, and not complaining about using integers. There's still a few undefined but referenced enums (ex: richtext.TextAttrDimensionFlags). These are most likely a union of some of the other flags/enum types already defined, but the work to pull that information from the C++ source is probably too much.
This fixes erroneous wx. prepended to `version` variable in methods' parameters.
Use `typing.Optional` and `typing.Union` where applicable, as direct union (`|`) type annotations were added in Python 3.10
Subscribing builtins.list wasn't added until Python 3.9 so use `typing.List` where applicable.
Subscripting builtins.tuple was added in Python 3.9, so use `typing.Tuple` where applicable.
`collections.abc.Callable` subscripting was added in Python 3.10, use `typing.Callable` instead.
lojack5
force-pushed
the
lojack-typed-typestubs
branch
from
bbd32c4
to
1bdc66a
Compare
swt2c
reviewed
Oct 18, 2023
| @@ -3,3 +3,4 @@ numpy < 1.17 ; python_version <= '2.7' | |||
| numpy ; python_version >= '3.0' and python_version < '3.12' | |||
| # pillow < 3.0 | |||
| six | |||
| typing-extensions; python_version < '3.10' | |||
There was a problem hiding this comment.
It looks like perhaps a space is needed before the semicolon?
There was a problem hiding this comment.
Syntactically not needed, but I'm more than happy to change it to match the style used in the rest of the file.
Currently working though issues shown to me through the buildbot, so I'll get to it after I solve all of those issues that come up.
BTW, I was looking for a CONTRIBUTING.md or a style guide somewhere but didn't find anything, am I missing it?
|
On October 18, 2023 6:10:28 PM EDT, Lojack ***@***.***> wrote:
@lojack5 commented on this pull request.
> @@ -3,3 +3,4 @@ numpy < 1.17 ; python_version <= '2.7'
numpy ; python_version >= '3.0' and python_version < '3.12'
# pillow < 3.0
six
+typing-extensions; python_version < '3.10'
Syntactically not needed, but I'm more than happy to change it to match the style used in the rest of the file.
Oh..I assumed that's why the CI was unhappy with it.
Currently working though issued shown to me through the buildbot, so I'll get to it after I solve all of those issues that come up.
BTW, I was looking for a CONTRIBUTING.md or a style guide somewhere but didn't find anything, am I missing it?
There isn't one AFAIK.
|
Alternate solution is to remove the callable typing on CallAfter and CallLater
lojack5
force-pushed
the
lojack-typed-typestubs
branch
from
d395ee0
to
12cbe9b
Compare
Since this is used to not only generate the type-stubs, but also the actual wx/core.py, we need to ensure TypeVar and ParamSpec are imported there as well. So, move the imports to the wx/core.py generator definition, and remove the imports from the type-stub generator (these imports are only used by CallAfter and CallLater).
lojack5
force-pushed
the
lojack-typed-typestubs
branch
from
12cbe9b
to
17cbd02
Compare
|
Ok, I think this is ready for review and whatever fixes you want from me. The CI is failing on all Windows builds - appears to be a MSVC version mismatch?
For reference, on my machine it builds fine for Python 3.12, Win 11 with the latest MSVC build tools installed. Buildbot output (for Python 3.11): vs what I see on my machine: The only thing that sticks out to me is the buildbot is calling into executables found in |
|
@swt2c sorry to bug. Are you able to review this (or help track down the windows compile error)? Or maybe know who to ping for it? |
Yes, I will try to track down the windows compiler error eventually. Too much to do and too little time.. :( |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Not sure what you did on the backend, but now everything except Python 3.7 are succeeding, nice! I looked into that, there wasn't even a build run for Python 3.7, so maybe a mismatch between what the GitHub check wants and what Azure was commanded to build? |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
??? |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Yeah, I fixed the CI builds and also removed Python 3.7 as it is EOL. Your old builds with Python 3.7 before that removal are still there. So at least your changes all build. Unfortunately, I think these changes probably need to be reviewed by @RobinD42 before merging. |
|
Awesome :). Thanks for the work getting that cleared up (and the thread cleanup). Hopefully Robin has some time in the future to look at this |
|
Just checking in to see if any dev's have time to review this. |
|
Thank you LoJack and especially thank Scott for cleanup. I do not see
anything abnormal in the stubs. Tomorrow or the next day I might... Lol.
…On Sun, Nov 12, 2023, 11:08 PM Lojack ***@***.***> wrote:
Just checking in to see if any dev's have time to review this.
—
Reply to this email directly, view it on GitHub
<#2468 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDTXRBLJYWE7YY7PC55MSLYEGTODAVCNFSM6AAAAAA6F72FX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGQ4DMMJQGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
... About the _ type _ stuff... This doesn't have anything to do with most
of that tho the "random" underscores might need to be watched. Generators
are known to jump out of documentation strings, and into code, which at
that point it becomes destructive.
On Tue, Dec 5, 2023, 7:48 AM Metalio Bovinus ***@***.***>
wrote:
… Thank you LoJack and especially thank Scott for cleanup. I do not see
anything abnormal in the stubs. Tomorrow or the next day I might... Lol.
On Sun, Nov 12, 2023, 11:08 PM Lojack ***@***.***> wrote:
> Just checking in to see if any dev's have time to review this.
>
> —
> Reply to this email directly, view it on GitHub
> <#2468 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABDTXRBLJYWE7YY7PC55MSLYEGTODAVCNFSM6AAAAAA6F72FX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGQ4DMMJQGI>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>
|
|
Checking back in on this, any chance for a dev to take a look at this? |
|
What do you want to test? I'm not sure. Python editors or dynamic python
editors? You can use the interactive interpreter or Sourcecoder. Otherwise,
you could use PyCharm, or any other editor NOT written in python. Yes I
know it sucks choosing an editor and not all of them are open source, so
what you want to do is sometimes colleged behind a idea that has been there
for years, but has not yet surfaced, well... publicly.
I'm just asking for your specs on how to do your "idea".
…On Sat, Feb 24, 2024, 11:14 PM Lojack ***@***.***> wrote:
Checking back in on this, any chance for a dev to take a look at this?
—
Reply to this email directly, view it on GitHub
<#2468 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDTXRDJ6FDEKJQC624TT3TYVLCCPAVCNFSM6AAAAAA6F72FX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSHAYTKNRXGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The wxPython type-stubs are nice in that they expose what classes, methods, etc are available, but there is no type information in them (there's even a comment in
makePyArgsStringreferencing implementing this). This is my 90% of the way there implementation of exposing this type information to the type-stubs.I also didn't find any open issues regarding type-stubs, so I haven't linked an issue.
For the most part, all of the type-information was already there in the doxygen files, it just had to be pulled into the args strings. There are a few exceptions, mostly revolving around types that are C++ typedefs (ex: WindowID and Coord).
By using
from __future__ import annotations, all of the type-hints are stringized, so even in the cases where the name referenced is undefined (ex: richtext's TextAttrDimensionFlags), the type-stub files are still valid. The hints in those cases will just provide no information.I do have a few discussion points to probably resolve before this is truly ready though:
typing.ParamSpec: related to the above, ParamSpec was added in Python 3.10. Currently it's only used to type-hintCallAfterandCallLater, how would you like this import handled:enums. I went the route of using the stdlib'senum.IntEnumandenum.IntFlaghere. My reasoning is - they're still typed asints this way (so compatible), and I define the enums with a prefixed_so the type-stubs don't expose them as some sort of real type that is actually accessible in actual wxPython code. I then make a TypeAlias for the name that includes the enum values or an int, so type-checkers won't complain to users that pass raw ints into methods typed with these enums. The enum then basically serves the purpose of grouping various values together and signaling the the callers which of the named values are expected at the call sites. I can go back to having enums just being typed as plain oldint, but I feel we lose the information of "what enum value or flags should you be using here".I also just realized while writing this that subscripting
listand unions with|weren't added until Python 3.9 and 3.10, respectively, so some for me:list[...]withList[...]and importListfromtyping.X | YwithUnion[X, Y]and importUnionfromtyping.tuple[...]withTuple[...]and importTuplefromtyping.from collections.abc import Callablewithfrom typing import Callable- subscripting was introduced in Python 3.10