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

Enable strict optional for more test files (9) #15607

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Jul 6, 2023

Followup to #15586

This will be the last one 🚀

@cdce8p cdce8p marked this pull request as ready for review July 7, 2023 13:55
@cdce8p cdce8p mentioned this pull request Jul 7, 2023

reveal_type(u(C(), None)) # N: Revealed type is "__main__.C"
reveal_type(u(None, C())) # N: Revealed type is "__main__.C"
reveal_type(u(C(), None)) # N: Revealed type is "Union[None, __main__.C]"
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to keep this test non-strict-optional? Seems like it specifically tests that None isn't kept as part of the union.

@@ -277,15 +278,15 @@ T = TypeVar('T')
S = TypeVar('S')
def u(x: T, y: S) -> Union[S, T]: pass

a = None # type: Any
a: Any

# Base-class-Any and None, simplify
Copy link
Member

Choose a reason for hiding this comment

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

This comment no longer applies

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the quick review!

I added --no-strict-optional here, similar to the one above. When we remove the flag at some point, we'll need to update all those test results anyway. For now it doesn't really matter tbh.

The goal is just to enable strict optional by default for test cases.

@JelleZijlstra JelleZijlstra merged commit d106c84 into python:master Jul 7, 2023
@cdce8p cdce8p deleted the test-optional-9 branch July 7, 2023 16:13
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