-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
docker: Fix some incomlpete types in docker #14817
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
base: main
Are you sure you want to change the base?
docker: Fix some incomlpete types in docker #14817
Conversation
This comment has been minimized.
This comment has been minimized.
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, looks mostly good. A few remarks.
# TODO: handle requires the type pywin32._win32typing.PyHANDLE | ||
def __init__(self, handle=None) -> 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.
Another workaround is to add a type alias like this above and use that here:
_PyHANDLE: TypeAlias = Any # TODO: pywin32._win32typing.PyHANDLE
# TODO: return type requires the type pywin32._win32typing.PyHANDLE | ||
def detach(self) -> 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.
Same here.
def check_resource(resource_name: str): ... | ||
def minimum_version(version: str): ... | ||
def update_headers(f: Callable[..., Incomplete]): ... | ||
def update_headers(f: Callable[..., 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.
Considering that this is a decorator, we should either use type var and use the same type var for the returned callable, or just leave it as Incomplete
for now.
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 used paramspec + typevar in the end :)
stubs/docker/docker/utils/utils.pyi
Outdated
def parse_devices(devices: Iterable[str | dict[str, Incomplete]]) -> list[dict[str, Incomplete]]: ... | ||
def kwargs_from_env(environment: Mapping[str, Incomplete] | None = None) -> _EnvKWArgs: ... | ||
def parse_devices(devices: Iterable[str | dict[str, str]]) -> list[dict[str, str]]: ... | ||
def kwargs_from_env(environment: Mapping[str, AnyStr] | None = None) -> _EnvKWArgs: ... |
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.
AnyStr
is a type var and as such needs a second use. You probably just want to use str | bytes
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.
did some more searching around and it seems it is only used as str
and not bytes | str
:)
stubs/pywin32/win32/win32pipe.pyi
Outdated
def GetOverlappedResult(hFile: int, overlapped: _win32typing.PyOVERLAPPED, bWait: int | bool, /) -> int: ... | ||
def WaitNamedPipe(pipeName: str, timeout, /) -> None: ... | ||
def GetNamedPipeInfo(hNamedPipe: int, /) -> tuple[Incomplete, Incomplete, Incomplete, Incomplete]: ... | ||
def GetNamedPipeInfo(hNamedPipe: int, /) -> tuple[int, int, int, int]: ... |
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.
Could we move this change to a separate PR?
Thank you for the detailed review! |
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Didn't do all of them but this is hopefully a nice start :)