Skip to content

Add platform checks for various sys values #4137

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

Merged
merged 3 commits into from
May 29, 2020

Conversation

CraftSpider
Copy link
Contributor

No description provided.

@CraftSpider
Copy link
Contributor Author

I think the failure is coming from this line of the mypy tests: https://github.com/python/mypy/blob/5cfb0561e624b79365ac5333cbde7ff500249740/test-data/stdlib-samples/3.2/test/support.py#L476

The test isn't specifically for platform, so the usage of a platform-specific function is invalid. Does anyone with more knowledge of mypy test suite know?

@CraftSpider CraftSpider requested a review from hauntsaninja May 28, 2020 23:11
@hauntsaninja
Copy link
Collaborator

Ugh, that's annoying. I think I might have actually tried to make this change before.

mypy seems pretty receptive to removing stdlib samples that are showing their age, see python/mypy#8838 (comment). I think test.support might be tricky to remove though, in which case maybe just change the check to be against sys.platform (or type-ignore that line or unconditionally define those constants).

@JelleZijlstra
Copy link
Member

Yes, probably easiest to make the mypy code use sys.platform instead as @hauntsaninja said. I can merge a mypy PR doing that.

@CraftSpider
Copy link
Contributor Author

Opened a PR to mypy as python/mypy#8916

@hauntsaninja hauntsaninja merged commit d035ce5 into python:master May 29, 2020
@CraftSpider CraftSpider deleted the sys-stubtest branch May 29, 2020 02:01
vishalkuo pushed a commit to vishalkuo/typeshed that referenced this pull request Jun 26, 2020
* Add platform checks for various sys values

* Add windows-only constants
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.

3 participants