Remove, move or `# noqa` more `TypeAlias` declarations #8450

AlexWaygood · 2022-07-31T09:31:28Z

Akuli · 2022-07-31T09:33:06Z

stdlib/multiprocessing/__init__.pyi

@@ -62,27 +60,6 @@ __all__ = [
 if sys.version_info >= (3, 8):
    __all__ += ["parent_process"]

-# The following type aliases can be used to annotate the return values of
-# the corresponding functions. They are not defined at runtime.


These were a bit controversial when @srittau added them, and he probably doesn't want them removed: #5346

Ah, thanks for the reference.

The other options are:

Special-case __init__.pyi files in the flake8-pyi check (what I did originally, but @JelleZijlstra doesn't like that).

Just # noqa all the ones in multiprocessing.

Abandon the flake8-pyi check entirely.

Something else?

I think this is somewhat of a "true positive". These type aliases are just weird: named as if they were private, intended to be public, not documented, not discoverable. But again, @srittau will probably disagree. I think we should wait for his input.

But again, @srittau will probably disagree. I think we should wait for his input.

Yeah of course, not going to merge without it :)

srittau · 2022-07-31T14:28:39Z

stdlib/multiprocessing/__init__.pyi

+# Using them as annotations is deprecated. Use imports from multiprocessing.queues instead.
+# See #4266 for discussion.


I think this comment is just outdated? As far as I can see, all these are classes in the implementation, at least in Python 3.7+. Was this maybe different in the past? In that case, I don't think we need the type aliases below.

They became functions in the stub at some point, but it was reverted.

Somehow I missed that you asked about the implementation, not about the stub. These are functions (actually methods) in the implementation:

>>> from multiprocessing import Queue >>> Queue <bound method BaseContext.Queue of <multiprocessing.context.DefaultContext object at 0x7f66f26d73d0>>

There is also an undocumented class that is different from the public and documented Queue:

>>> from multiprocessing.queues import Queue >>> Queue <class 'multiprocessing.queues.Queue'>

There's similarly e.g. a method multiprocessing.Event and a class multiprocessing.synchronize.Event.

This is what happens at runtime:

>>> import multiprocessing >>> multiprocessing.Queue <bound method BaseContext.Queue of <multiprocessing.context.DefaultContext object at 0x000002F32AF7E5C0>> >>> multiprocessing.Queue[int] Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: 'method' object is not subscriptable

So the stubs are a bit of a lie at the moment. As @Akuli says, the stubs were changed in #4289 so that they could match the runtime more closely, but then that was reverted in #4314 after Jukka complained in #4313.

The thing is, either way, I don't think having these aliases helps the situation much. To work around these issues, a user can probably just put multiprocessing.Queue in quotes in their annotations, or use from __future__ import annotations.

We should try to make them into functions again in the stubs and see the fallout, now that we have mypy_primer. Annotating something with muliprocessing.Queue, which is a function, is not something we should support.

I marked my previous comment as off-topic, as it doesn't belong at all to this PR. I somehow didn't see other people's comments that were posted before I wrote mine.

since the type aliases are connected to this, we should keep those that are alternatives to a function.

I don't really see how the aliases help, even the ones which are alternatives to a function.

Take multiprocessing.Lock -- multiprocessing.Lock is a method, but multiprocessing.synchronize.Lock is a class. If it's a choice between doing this:

from multiprocessing.synchronize import Lock foo: Lock

or this:

from typing import TYPE_CHECKING if TYPE_CHECKING: from multiprocessing import _LockType foo: "_LockType"

Then, I find the first way much cleaner and more ergonomic. And, as @Akuli says in his minimized comment, we know none of the projects in mypy_primer are using these aliases.

Sorry for being unclear. Yes, if there is a valid type somewhere, like multiprocessing.synchronize.Lock, we don't need the alias. It's possible that this is true for all aliases, I didn't check.

_BarrierType, _BoundedSemaphoreType, _ConditionType, _EventType, _LockType, _RLockType and _SemaphoreType are all aliases to valid types in multiprocessing.synchronize.

_QueueType, _SimpleQueueType and _JoinableQueueType are all aliases to valid types in multiprocessing.queues.

So yeah, it's true for all the aliases :)

AlexWaygood · 2022-08-05T11:31:05Z

So, are we happy for this to go in? :)

stdlib/multiprocessing/__init__.pyi

github-actions · 2022-08-05T11:45:24Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Remove, move or # noqa more TypeAlias declarations

571e375

Akuli reviewed Jul 31, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

srittau reviewed Jul 31, 2022

View reviewed changes

AlexWaygood commented Aug 5, 2022

View reviewed changes

stdlib/multiprocessing/__init__.pyi Outdated Show resolved Hide resolved

AlexWaygood and others added 2 commits August 5, 2022 12:36

Improve comment

1424f10

[pre-commit.ci] auto fixes from pre-commit.com hooks

afbf143

srittau approved these changes Aug 5, 2022

View reviewed changes

AlexWaygood merged commit 892796a into master Aug 5, 2022

AlexWaygood deleted the more-aliases branch August 5, 2022 12:03

sobolevn mentioned this pull request Nov 15, 2022

gh-99509: Add __class_getitem__ to multiprocessing.Queue python/cpython#99511

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove, move or `# noqa` more `TypeAlias` declarations #8450

Remove, move or `# noqa` more `TypeAlias` declarations #8450

AlexWaygood commented Jul 31, 2022

Akuli Jul 31, 2022

AlexWaygood Jul 31, 2022

AlexWaygood Jul 31, 2022

Akuli Jul 31, 2022

AlexWaygood Jul 31, 2022 •

edited

Loading

This comment has been minimized.

srittau Jul 31, 2022

Akuli Jul 31, 2022 •

edited

Loading

Akuli Jul 31, 2022

AlexWaygood Jul 31, 2022 •

edited

Loading

srittau Jul 31, 2022

This comment was marked as off-topic.

Akuli Jul 31, 2022

AlexWaygood Jul 31, 2022 •

edited

Loading

srittau Jul 31, 2022

AlexWaygood Jul 31, 2022 •

edited

Loading

AlexWaygood commented Aug 5, 2022

github-actions bot commented Aug 5, 2022

		# Using them as annotations is deprecated. Use imports from multiprocessing.queues instead.
		# See #4266 for discussion.

Remove, move or # noqa more TypeAlias declarations #8450

Remove, move or # noqa more TypeAlias declarations #8450

Conversation

AlexWaygood commented Jul 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexWaygood Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Akuli Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexWaygood Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

Choose a reason for hiding this comment

AlexWaygood Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexWaygood Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

AlexWaygood commented Aug 5, 2022

github-actions bot commented Aug 5, 2022

Remove, move or `# noqa` more `TypeAlias` declarations #8450

Remove, move or `# noqa` more `TypeAlias` declarations #8450

AlexWaygood Jul 31, 2022 •

edited

Loading

Akuli Jul 31, 2022 •

edited

Loading

AlexWaygood Jul 31, 2022 •

edited

Loading

AlexWaygood Jul 31, 2022 •

edited

Loading

AlexWaygood Jul 31, 2022 •

edited

Loading