-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stubs replace : Any | None
with : Incomplete | None
#9565
Stubs replace : Any | None
with : Incomplete | None
#9565
Conversation
This comment has been minimized.
This comment has been minimized.
@@ -191,7 +191,7 @@ class Values(Generative, FromClause): | |||
name: Any | |||
literal_binds: Any | |||
def __init__(self, *columns, **kw) -> None: ... | |||
def alias(self: Self, name: Any | None, **kw) -> Self: ... # type: ignore[override] | |||
def alias(self: Self, name: Incomplete | None, **kw) -> Self: ... # type: ignore[override] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the ones in this file are stubgen artefacts; they were all added in https://github.com/python/typeshed/pull/7603/files. But, there's no source-code comment indicating why they used Any
here (and the comments on the PR thread give no insight), so I think it's fine to change the Any
uses in this file to Incomplete
.
(Same comment applies to a few other files touched by this PR.)
|
||
class _SupportsGraphident(Protocol): | ||
graphident: str | ||
|
||
# code, filename and packagepath are always initialized to None. But they can be given a value later. | ||
class Node: | ||
# Compiled code. See stdlib.builtins.compile | ||
code: Any | None | ||
code: Incomplete | None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only added this one recently and, from the comment, it looks like it's meant to be Any
;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely, I don't see anything else obvious after manually reviewing as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't checked the code but it looks like this should just be types.CodeType
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR no longer touches this line, so I think that can be deferred to another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the return of stdlib/builtins.pyi > compile
, which is currently typed as Any
, but also has this comment:
TODO:
compile
has a more precise return type in reality; work on a way of expressing that?
https://github.com/python/typeshed/blob/main/stdlib/builtins.pyi#L1234
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I'd be surprised if PyInstaller is using PyCF_ONLY_AST here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(technically it's modulegraph, pyinstaller vendors it) It is using PyCF_ONLY_AST as well, but it's the non-ast call that is stored.
# TODO: change this to packaging.markers.Marker | None once we can import | ||
# packaging.markers | ||
marker: Any | None | ||
marker: Incomplete | None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, we should, you know, take action on this comment now (in a separate PR)
(The only actionable thing in my review is the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
First commit does
: Any | None\n
to: Incomplete | None\n
Second commit does
: Any | None
to: Incomplete | None
Split them up for ease of review, just in case. With #9558 I think we can effectively say that all
: Any | None
were to be replaced.The only instances of
Any | None
left are ClassVar type parameter, return types and 2 Aliases because of name clash:tAny | None
in stubs\protobuf\google\protobuf\internal\well_known_types.pyi_Any | None
in stubs\SQLAlchemy\sqlalchemy\dialects\postgresql\array.pyiRef #9550