Skip to content

For Optional[T] or T, infer the type T instead of Optional[T]. #1818

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

Closed
wants to merge 3 commits into from

Conversation

gvanrossum
Copy link
Member

Fixes #1817, fixes #1733.

@gvanrossum
Copy link
Member Author

Is there an equivalent example for and? I can't think of any.

FWIW this is kind of blocking further explorations of --strict-optional on mypy itself.


[case testOptionalTypeOrTypePlain]
from typing import Optional
def f(a: Optional[int]) -> int:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe test or with both operands optional?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, could test left operand with a complex union type such as Union[int, str, None].

Copy link
Member Author

Choose a reason for hiding this comment

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

@ddfisher
Copy link
Collaborator

ddfisher commented Jul 7, 2016

I feel like we shouldn't need this special casing and this should just fall out of the find_isinstance_check like everything else. I'm a bit surprised that that isn't the case already -- going to briefly investigate why that is.

@ddfisher
Copy link
Collaborator

ddfisher commented Jul 8, 2016

Looks like that was actually pretty straightforward to do. Let's consider #1828 instead of this?

@gvanrossum
Copy link
Member Author

Closing in favor of #1828.

@gvanrossum gvanrossum closed this Jul 8, 2016
@gvanrossum gvanrossum deleted the optional-x-or-x branch July 8, 2016 00:33
@gvanrossum
Copy link
Member Author

Cool. Maybe someday I'll understand how this works -- the name (find_isinstance_check()) and the resulting variables (right_map, left_map -- why the transposition?) were all kind of confusing.

@ddfisher
Copy link
Collaborator

ddfisher commented Jul 8, 2016

Agree it's all pretty confusing. The function might be be better named find_conditional_type_information or similar. The transposition is because for and we care about the type of the left hand expression if it's falsey but for or we care about the type of the left hand expression when it's truthy (and find_isinstance_check returns a (conditional type map if true, conditional type map if false) tuple).

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.

Improve type inference for "Optional[T] or T" expression Type inference of or and Optional[x]
3 participants