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.

Sign up for GitHub

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

Jump to bottom

update ellipsis type #11564

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

update ellipsis type #11564

wants to merge 2 commits into from

Conversation

DetachHead
Copy link
Contributor

@DetachHead
Copy link
Contributor Author

i have no idea what i'm doing lol

sobolevn
sobolevn reviewed Nov 19, 2021
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Can you please explain your idea behind this PR? Maybe I can help you 🙂

mypy/typeshed/stdlib/_typeshed/__init__.pyi
@@ -184,3 +184,7 @@ else:
@final
class NoneType:
def __bool__(self) -> Literal[False]: ...

# Actually the type of Ellipsis is <type 'ellipsis'>, but since it's
Copy link
Member

Choose a reason for hiding this comment

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

This file is just synced from here: https://github.com/python/typeshed/blob/master/stdlib/_typeshed/__init__.pyi

Manually updating it is not a good practice.

mypy/checkexpr.py
@@ -2160,7 +2160,7 @@ def visit_complex_expr(self, e: ComplexExpr) -> Type:
def visit_ellipsis(self, e: EllipsisExpr) -> Type:
"""Type check '...'."""
if self.chk.options.python_version[0] >= 3:
return self.named_type('builtins.ellipsis')
return self.named_type('_typeshed.ellipsis')
Copy link
Member

Choose a reason for hiding this comment

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

We don't use _typeshed. types in mypy right now. It would be hard to test.

@DetachHead
Copy link
Contributor Author

@sobolevn i'm trying to make ellipsis private to fix #11559 but i have no idea how. apparently it's a more difficult problem than i thought? see python/typeshed#3556

@sobolevn
Copy link
Member

Yes, this is going to be hard 🙂

What I can see here:

  1. A way to make _typeshed discoverable during module graph builder
  2. Maybe we would need to special case it in semanal.py as we do with typing and typing_extensions
  3. We need to think how to test it. Right now mypy uses the bare minimum for each test. This is made to run tests faster. 3rd party modules are not supported. So, probably we would either need to allow _typeshed or to use an extra fixture.
  4. Finally, we need to consult core-devs if everything is right. This is going to be a really big change! But, a very useful one. There are other cases when we can use _typeshed types: NoneType, SupportsWrite, etc.

@JelleZijlstra JelleZijlstra mentioned this pull request Apr 3, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobolevn sobolevn sobolevn left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@DetachHead @sobolevn