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

Improve error message for partial None with --local-partial-types #12822

Merged
merged 5 commits into from
Sep 2, 2022

Conversation

pranavrajpal
Copy link
Contributor

Description

Suggest an annotation of the form Optional[<type>] when we show an error for an inferred partial None type (which I think should only happen when --local-partial-types is set).

Should help with merging #10169 (see #10169 (comment)).

Test Plan

The error message in question appeared in a few tests which got changed. I think that should be sufficient to test this since this isn't a particularly complicated change.

When --local-partial-types is set and we can't infer a complete type for
a type that we initially inferred as partial None, show an error message
that suggests to add a type annotation of the form Optional[<type>].
@github-actions

This comment has been minimized.

Fix error messages in fine grained tests
@github-actions

This comment has been minimized.

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.

Nice! Looks good. It would be good to suggest the modern union syntax when it's supported.

test-data/unit/check-inference.test Show resolved Hide resolved
@github-actions

This comment has been minimized.

Code running on Python 3.10 can use PEP 604 annotations, so suggest
those instead.
@github-actions

This comment has been minimized.

Copy link
Collaborator

@ethanhs ethanhs left a comment

Choose a reason for hiding this comment

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

The new code is much cleaner!

mypy/messages.py Outdated
@@ -1193,18 +1193,30 @@ def need_annotation_for_var(self, node: SymbolNode, context: Context,
python_version: Optional[Tuple[int, int]] = None) -> None:
hint = ''
has_variable_annotations = not python_version or python_version >= (3, 6)
pep604_supported = python_version and python_version >= (3, 10)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: feels like this should mirror the check for has_variable_annotations when python_version is None

@github-actions

This comment has been minimized.

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

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

@hauntsaninja hauntsaninja merged commit cf7495f into python:master Sep 2, 2022
@pranavrajpal pranavrajpal deleted the improve-partial-None-error branch September 2, 2022 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants