Skip to content
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

Add multiprocessing.sharedctypes stubs #5231

Merged
merged 12 commits into from
May 2, 2021
Merged

Conversation

hatal175
Copy link
Contributor

Fixes #4047.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip.git)
+ zerver/lib/test_runner.py:208: error: Name 'multiprocessing.sharedctypes._Value' is not defined  [name-defined]

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick note in lieu of a full review: I think pytype now supports bound methods, so we should be able to replace a lot of the duplication in multiprocessing.__init__

@hatal175
Copy link
Contributor Author

pyright gave me this issue:
https://github.com/hatal175/typeshed/runs/2377568497?check_suite_focus=true
which I could not overcome - that's why I basically copied all the functions to __init__.

@hauntsaninja
Copy link
Collaborator

Thanks, yeah, this is the pytype issue: google/pytype#761 Looks like the fix was only for bound methods within a file

@rchen152
Copy link
Collaborator

The fix in that pytype issue was supposed to be a complete one, not just for within a file. (I realize now that it looks confusing because I didn't actually add another comment to the issue, but you can see in the history that it was auto-closed by google/pytype@0a50abf a couple months after the partial fix.)

Playing around with this PR, it looks like I missed a couple things - the aliased methods aren't found when they're from a base class, and there are some import resolution issues depending on the style of import used. I'll see if I can do a quick fix and open another pytype issue to track the problems if not.

@rchen152
Copy link
Collaborator

I wasn't able to figure out the import resolution issues, but I did get base class lookup working, so after the next pytype release, you should be able to do:

from multiprocessing import context
Array = context._default_context.Array
# etc.

Unfortunately, you have to use this exact style of import for pytype to resolve things correctly; from multiprocessing.context import _default_context or other variations don't work.

rchen152 added a commit to google/pytype that referenced this pull request Apr 23, 2021
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip.git)
+ zerver/lib/test_runner.py:208: error: Name 'multiprocessing.sharedctypes._Value' is not defined  [name-defined]

2 similar comments
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip.git)
+ zerver/lib/test_runner.py:208: error: Name 'multiprocessing.sharedctypes._Value' is not defined  [name-defined]

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip.git)
+ zerver/lib/test_runner.py:208: error: Name 'multiprocessing.sharedctypes._Value' is not defined  [name-defined]

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. The mypy_primer failure can be ignored.

One general note: Please use built-in generics in Python-3-only stubs. E.g. type[...] instead of Type[...]. Also you can import other generics from collections.abc instead of typing. You don't need to change it for existing code (although you can), but something to keep in mind in the future.

More notes below.

Also please note that you should probably merge current master into this PR, update dependencies and re-run black as the formatting has changed in regards to overloads. Sorry about that!

@overload
def RawValue(self, typecode_or_type: Type[_CT], *args: Any) -> _CT: ...
@overload
def RawValue(self, typecode_or_type: Union[str, Type[_CData]], *args: Any) -> Any: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type[_CData] is already covered by the first overload and will never match in the second one. Same for RawArraw and the corresponding definitions in .sharedctypes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_CData is covered but the union is not - unless I'm missing something. So If I'm giving this Union[str, _CArray] I want this to return Any. Right?

Copy link
Collaborator

@srittau srittau May 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type checkers should be able to build a union themselves. For example with mypy:

from typing import overload, Union

@overload
def foo(x: str) -> str: ...
@overload
def foo(x: int) -> int: ...

x: Union[int, str]
reveal_type(foo(x))  # Revealed type is "Union[builtins.int, builtins.str]"

Edit: So in this case, Any would be part of the union, which is basically the same as writing Any, but has the additional advantage that IDEs etc. might suggest types appropriate for _CData.

@hatal175
Copy link
Contributor Author

hatal175 commented May 2, 2021

I did the collections.abc change and the list/tuple change on the way in these files but I couldn't make the Type change - stubtest complained. I am working on python 3.8 so there might be a mypy bug there.
Also note there isn't a single type[...] use in the repo.

@srittau
Copy link
Collaborator

srittau commented May 2, 2021

I did the collections.abc change and the list/tuple change on the way in these files but I couldn't make the Type change - stubtest complained. I am working on python 3.8 so there might be a mypy bug there.
Also note there isn't a single type[...] use in the repo.

There are unfortunately still a few mypy bugs around this.

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2021

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip.git)
+ zerver/lib/test_runner.py:208: error: Name 'multiprocessing.sharedctypes._Value' is not defined  [name-defined]

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2021

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip.git)
+ zerver/lib/test_runner.py:208: error: Name 'multiprocessing.sharedctypes._Value' is not defined  [name-defined]

@srittau srittau merged commit b15c025 into python:master May 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stub missing for multiprocessing.sharedctypes
4 participants