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

Supports __future__.annotations #7963

Merged
merged 8 commits into from
Sep 6, 2020
Merged

Supports __future__.annotations #7963

merged 8 commits into from
Sep 6, 2020

Conversation

TH3CHARLie
Copy link
Collaborator

@TH3CHARLie TH3CHARLie commented Nov 18, 2019 •
edited
Loading

This PR partially addresses #7907.

@TH3CHARLie
Copy link
Collaborator Author

TH3CHARLie commented Nov 18, 2019 •
edited
Loading

However, tests are lacked. I'm not sure about in which unit-test file should the test be created. @ilevkivskyi

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 19, 2019

You can add a new test file, since no existing file seems to be a very good fit. Maybe test-data/unit/check-future.test?

@TH3CHARLie
Copy link
Collaborator Author

You can add a new test file, since no existing file seems to be a very good fit. Maybe test-data/unit/check-future.test?

Good idea, I will get it done once I wake up tomorrow

JukkaL
JukkaL reviewed Nov 21, 2019
View reviewed changes
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

After thinking about this more, it's unclear how this should work exactly (and if anything needs to change). Let's discuss the desired semantics first in #7907.

test-data/unit/lib-stub/collections.pyi Outdated
_KT = TypeVar('_KT')
_VT = TypeVar('_VT')

class defaultdict(Dict[_KT, _VT], Generic[_KT, _VT]):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We try to keep test stubs as small as possible. Basically we only include things that are required by test cases. In this case, it would be sufficient to have a non-overloaded __init__, since the test case doesn't depend on most of the overload variants.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. I'll fix it once along with further changes if we have an agreement in #7907

@emmatyping
Copy link
Collaborator

emmatyping commented Apr 7, 2020 •
edited
Loading

@TH3CHARLie would you like to resurrect this PR?

I believe the semantics are better defined now.

@TH3CHARLie
Copy link
Collaborator Author

@TH3CHARLie would you like to resurrect this PR?

I believe the semantics are better defined now.

Great, I'll read through the new PEP and update the changes in this PR.

@emmatyping
Copy link
Collaborator

@TH3CHARLie note that the total set of things being supported is more than just those enumerated in the PEP.

@TH3CHARLie TH3CHARLie changed the title Understand __future__.annotations [WIP]Understand __future__.annotations Apr 10, 2020
@TH3CHARLie
Copy link
Collaborator Author

@ethanhs If I understand correctly, to support PEP585 in mypy, all items listed in gvanrossum/cpython#1 (comment) can now be parameterized, so this PR actually needs to spot such use cases and suppress currently reported errors, right?

@emmatyping
Copy link
Collaborator

@TH3CHARLie that is essentially correct. I think the best way to think of it is "anything in the stdlib that is Generic in typeshed". That may be easier to implement too.

@TH3CHARLie
Copy link
Collaborator Author

anything in the stdlib that is Generic in typeshed

I had one initial version now but it's simply enumerating all the cases using a long list so I am interested in you mentioned "anything in the stdlib that is Generic in typeshed". Can this be achieved through some interaction with typeshed so mypy computes the generic list automatically?(I'm actually not pretty familiar with typeshed codebase)

@TH3CHARLie TH3CHARLie changed the title [WIP]Understand __future__.annotations Implement PEP 585 (Generic builtins and __future__.annotations) Apr 14, 2020
@TH3CHARLie
Copy link
Collaborator Author

TH3CHARLie commented Apr 14, 2020 •
edited
Loading

Sorry for the delay. I carelessly forget to push my initial version but thanks to this I think twice and find my first attempt to suppress more errors than it should.

The PR now suppresses errors on generic built-ins, the only nontrivial change(compared to original version before PEP585 is accepted) is to handle type. @ethanhs

@TH3CHARLie TH3CHARLie requested a review from JukkaL April 14, 2020 18:53
@TH3CHARLie
Copy link
Collaborator Author

ping @JukkaL and @ethanhs for review

@AllanDaemon
Copy link
Contributor

Is this planned to be released before Python 3.9 release, that will implement PEP 585?

@TH3CHARLie
Copy link
Collaborator Author

Is this planned to be released before Python 3.9 release, that will implement PEP 585?

Thanks for asking! This PR is delayed since we spent a significant amount of time working on compiler kinds of stuff. I'll try to mention this with core developers and move this forward.

@emmatyping
Copy link
Collaborator

@TH3CHARLie I've read over the PR, but I believe as written it currently only supports __future__.annotations, not the full PEP 585, which adds support for sub-scripting builtins on 3.9+ without the future import. I think it might be better to land this and implement PEP 585 in another PR.

emmatyping
emmatyping approved these changes Sep 6, 2020
View reviewed changes
Copy link
Collaborator

@emmatyping emmatyping 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 for working on this!

@TH3CHARLie
Copy link
Collaborator Author

@TH3CHARLie I've read over the PR, but I believe as written it currently only supports __future__.annotations, not the full PEP 585, which adds support for sub-scripting builtins on 3.9+ without the future import. I think it might be better to land this and implement PEP 585 in another PR.

Thanks for the review! then I'll rename this PR and merge it.

@TH3CHARLie TH3CHARLie changed the title Implement PEP 585 (Generic builtins and __future__.annotations) Supports __future__.annotations Sep 6, 2020
@TH3CHARLie TH3CHARLie merged commit 720b77e into python:master Sep 6, 2020
@hauntsaninja hauntsaninja mentioned this pull request Sep 17, 2020
Closed
@huguesb huguesb mentioned this pull request Sep 24, 2020
Closed
This was referenced Oct 17, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emmatyping emmatyping emmatyping approved these changes

@JukkaL JukkaL Awaiting requested review from JukkaL

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

Successfully merging this pull request may close these issues.
4 participants
@TH3CHARLie @JukkaL @emmatyping @AllanDaemon