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

Introduce _typeshed.WeakUnion #7437

Closed
wants to merge 2 commits into from

Conversation

not-my-profile
Copy link
Contributor

No description provided.

@not-my-profile not-my-profile marked this pull request as draft March 4, 2022 13:25
@github-actions

This comment has been minimized.

@@ -10,6 +10,25 @@ from os import PathLike
from typing import AbstractSet, Any, Container, Generic, Iterable, Protocol, TypeVar, Union
from typing_extensions import Final, Literal, final

WeakUnion = Any
Copy link
Member

Choose a reason for hiding this comment

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

what if we do this:

Suggested change
WeakUnion = Any
class WeakUnion(Any):
def __class_getitem__(cls, args: Any) -> Any: ...

Copy link
Member

Choose a reason for hiding this comment

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

mypy doesn't (yet) have a generalised understanding of __class_getitem__. It only understands a class as being generic if the class inherits from typing.Generic. https://mypy-play.net/?mypy=latest&python=3.10&gist=1a3ed0af028c24c591ea1e63d80954f6

Copy link
Member

Choose a reason for hiding this comment

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

That's too bad. Putting __getitem__ on the metaclass doesn't work either. This might be easier to fix than full PEP 646 support though.

Copy link
Member

@AlexWaygood AlexWaygood Mar 4, 2022

Choose a reason for hiding this comment

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

I have reservations about teaching mypy about __class_getitem__, as I think __class_getitem__ should properly be seen as an implementation detail of the typing framework. But I agree that it would be nice for mypy to have a stronger understanding of metaclass __getitem__.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you give an example of what __getitem__ with a metaclass would look like? And is there any type type checker that currently handles that? I think we should open issues with mypy, pyright, pytype & pyre if they don't support that currently.

Copy link
Contributor Author

@not-my-profile not-my-profile Mar 5, 2022

Choose a reason for hiding this comment

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

using it in a return type doesn't work with pyright:

variable not allowed unless it is a type alias

Copy link
Collaborator

@hauntsaninja hauntsaninja Mar 5, 2022

Choose a reason for hiding this comment

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

Hmm. I guess maybe that's part of the "Unknown" vs "Any" distinction pyright has? The even dumber hack is from _typeshed.does_not_exist import WeakUnion as WeakUnion # type: ignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That doesn't help pyright still treats such types it cannot import as Unknown.

Copy link
Member

Choose a reason for hiding this comment

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

@erictraut, how hard would it be for you to implement special-casing in pyright for a WeakUnion/UnsafeUnion symbol imported from _typeshed? It wouldn't need to be the full implementation of this proposed feature just yet; it would just need to be a special form semantically identical to Any that could be provided with arbitrary subscriptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer not to do it that way. This approach looks very hacky to me, and I don't think it should go into typeshed this way.

If it's going to be a special form, then I think we should go through a PEP process and define it formally. If we're going to use PEP 646 as I suggested, then let's do that — although we'd need to wait for some minimal support in mypy first.

If this is just going to be an Any for now, then there's no benefit in the short term. I don't see the need to rush it. Let's pick a sound approach and get this right.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 4, 2022

By the way, if it's appropriate to bikeshed here, I have a big preference for a name with "Any" (or possible "Unsafe") in it. The principle being that it should be obvious to users when a type involves unsafety. So candidates include:

  • AnyOf
  • AnyUnion
  • UnsafeUnion

(my vote is currently still for AnyOf)

@AlexWaygood
Copy link
Member

By the way, if it's appropriate to bikeshed here, I have a big preference for a name with "Any" (or possible "Unsafe") in it. The principle being that it should be obvious to users when a type involves unsafety. So candidates include:

  • AnyOf
  • AnyUnion
  • UnsafeUnion

I like UnsafeUnion or WeakUnion. I agree that emphasising the unsafety is good, but I think emphasising the fact that it's like a union is also good.

@not-my-profile not-my-profile changed the title Introduce _typeshed.WeakUnion = Any Introduce _typeshed.WeakUnion Mar 8, 2022
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2022

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

@AlexWaygood
Copy link
Member

The test failures indicate that this would be very difficult to do without adding special-casing to type checkers to account for this feature. As such, I'm going to close this for now, but we can maybe revisit it after further discussion in the python/typing repo or the typing-sig mailing list.

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.

5 participants