Skip to content

Conversation

Akuli
Copy link
Collaborator

@Akuli Akuli commented Jun 21, 2021

Fixes python/mypy#8166 so that python/mypy#8461 is no longer needed.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

mypy (https://github.com/python/mypy.git)
+ mypyc/test/test_run.py:284: note: (Skipping most remaining errors due to unresolved imports or missing stubs; fix these first)

aiohttp (https://github.com/aio-libs/aiohttp.git)
+ aiohttp/helpers.py:218: error: Non-overlapping equality check (left operand type: "Literal['Linux']", right operand type: "Literal['Windows']")  [comparison-overlap]

werkzeug (https://github.com/pallets/werkzeug.git)
+ src/werkzeug/serving.py:70: error: Non-overlapping equality check (left operand type: "Literal['Linux']", right operand type: "Literal['Windows']")
- tests/test_wrappers.py:710: error: Function is missing a return type annotation

@Akuli
Copy link
Collaborator Author

Akuli commented Jun 21, 2021

Now that I see the mypy_primer output, I'm no longer sure about whether this is a good idea, because it now warns about Literal["Linux"] == Literal["Windows"].

@JelleZijlstra
Copy link
Member

Yes, that's unfortunate. Maybe that mypy option can be taught to not error when comparing two literals that aren't part of a union.

@srittau
Copy link
Collaborator

srittau commented Jun 21, 2021

This is an interesting case. Looked at from one side, this change is correct. Looked at from another side it isn't. I would suggest for now not to use conditionals and just return Literal['Linux', 'Windows', 'Darwin'] | str until mypy is changed. After that, I think this PR is worth a try.

@Akuli Akuli added the status: deferred Issue or PR deferred until some precondition is fixed label Jun 21, 2021
@JelleZijlstra
Copy link
Member

Going to close this as stale for now to keep the list of open PRs manageable. If you're still interested in seeing this through, feel free to reopen or open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: deferred Issue or PR deferred until some precondition is fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recognize platform.system() as well as sys.platform
3 participants