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

Allow Annotated to wrap NotRequired in a TypedDict definition. #15852

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bsmedberg-xometry
Copy link

Currently, it is impossible to annotate a non-required TypedDict field using Annotated because mypy complains that NotRequired can only be used in field definitions.

This PR relaxes the requirement such that you can use Annotated[NotRequired[str], "annotation"]. I've verified that the type-checking behavior is correct in this case.

@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@bsmedberg-xometry
Copy link
Author

@davidfstr you've helped me with mypy PRs before and this is related (but much smaller/easier). Can you help shepherd this?

@davidfstr
Copy link
Contributor

davidfstr commented Aug 14, 2023

Thanks for providing the PR! Looking like it implements part of the PEP 655 (NotRequired) that was missed.

There are a lot of items on my plate right now, so the earliest I can take a look is this weekend starting 8/19.

@davidfstr
Copy link
Contributor

This change is quite straightforward. LGTM.

An explanation for other reviewers:

  • (Not)Required[] is only permitted in certain contexts within type annotation expressions, and is disallowed everywhere else by default. In particular (Not)Required[] is generally only permitted at the top-level.
  • (1) This PR alters TypeAnalyser.try_analyze_special_unbound_type's behavior when it is examining an Annotated[T] so that when it sub-analyzes T using TypeAnalyser.anal_type(), it passes allow_required=True, so that T is allowed to be a (Not)Required[] expression.
  • (2) This PR alters TypeAnalyser.anal_type() with a new allow_required parameter that can override anal_type() to allow (Not)Required[] one level deeper (but no further).
    • Previously anal_type() would never allow (Not)Required[] at deeper nesting levels.

@davidfstr
Copy link
Contributor

davidfstr commented Aug 23, 2023

@bsmedberg-xometry , could you rebase this PR on the tip of master?

Afterwards I'd suggest tagging the recent mergers (hauntsaninja, ilevkivskyi, sobolevn), saying this PR is ready to merge.

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.

2 participants