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

Recognize platform.system() as well as sys.platform #8166

Open
brettcannon opened this issue Dec 17, 2019 · 11 comments · May be fixed by #10686
Open

Recognize platform.system() as well as sys.platform #8166

brettcannon opened this issue Dec 17, 2019 · 11 comments · May be fixed by #10686
Labels

Comments

@brettcannon
Copy link
Member

Note: if you are reporting a wrong signature of a function or a class in
the standard library, then the typeshed tracker is better suited
for this report: https://github.com/python/typeshed/issues

Please provide more information to help us understand the issue:

  • Are you reporting a bug, or opening a feature request? feature request
  • Please insert below the code you are checking with mypy,
    or a mock-up repro if the source is private. We would appreciate
    if you try to simplify your case to a minimal repro.
    if platform.system() != "Windows":  # sys.platform != "win32":
        try:
            # os.confstr("CS_GNU_LIBC_VERSION") returns a string like "glibc 2.17".
            version_string = os.confstr("CS_GNU_LIBC_VERSION")
            assert version_string is not None
            _, version = version_string.split()
        except (AssertionError, OSError, ValueError):
            # os.confstr() or CS_GNU_LIBC_VERSION not available (or a bad value)...
            return None
        return version
    return None
  • What is the actual behavior/output? "error: Module has no attribute "confstr""
  • What is the behavior/output you expect? No error when run under Windows
  • What are the versions of mypy and Python you are using? mypy 0.750, Python 3.7.4

Basically it would be nice to get to use platform.system() instead of having to use sys.platform to make mypy happy with code that is conditional on the platform.

@ilevkivskyi
Copy link
Member

On one hand this is pretty niche, but on other hand should be not hard to implement.

@neesara
Copy link

neesara commented Dec 20, 2019

Hi @ilevkivskyi I am interested in working on this issue, please guide me on where I can start.

@ilevkivskyi
Copy link
Member

Look at consider_sys_platform() in mypy/reachability.py.

@neesara
Copy link

neesara commented Jan 1, 2020

Which option would you recommend? - Write a new function similar to 'consider_sys_platform()' for handling platform.system() or modify 'consider_sys_platform()' to accomodate platform.system()

@JelleZijlstra JelleZijlstra changed the title Recognize system.platform() as well as sys.platform is Recognize platform.system() as well as sys.platform is Jan 2, 2020
@joybh98
Copy link
Contributor

joybh98 commented Feb 28, 2020

@ilevkivskyi , I was reading the function consider_sys_platform, and from what I could understand, we want to check Comaprision Expression, and call expressions using the platform.system() module.
In addition to the existing sys.platform(). Am I on the right track?

@ilevkivskyi
Copy link
Member

Yes.

@joybh98
Copy link
Contributor

joybh98 commented Mar 2, 2020

@ilevkivskyi I was going through the code, and I think I should:

  1. Change function name from consider_sys_platform to consider_platform
  2. Change platform to sys.platform and add platforms in options.py (I've already done that in the PR, but I have to do a lot of work as it failed all the tests)
  3. Update tests to accommodate reflecting changes

Please let me know if this approach is right or not.

@ilevkivskyi
Copy link
Member

No, you need to change the "parsing" logic to recognize the platform.system() as well.

Btw, no need to ask me about this. There are other team members who can review your PR.

@joybh98
Copy link
Contributor

joybh98 commented Mar 7, 2020

@ALL please take a look at the PR and let me know what I am doing wrong, would really like your input.

@atakiar atakiar linked a pull request Jun 21, 2021 that will close this issue
gsnedders added a commit to web-platform-tests/wpt that referenced this issue Oct 24, 2021
This:

 * Deals with changes in how mypy handles metaclasses

 * Prefers sys.platform == "win32" due to
   python/mypy#8166 and mypy not having
   WindowsError defined by default any more

 * Installs various typestubs

 * Rewrites tox.ini to avoid duplicating everything, and allowing new
   versions of Python to be easily tested (as tox -e py310-mypy will
   now work without further changes).
foolip pushed a commit to web-platform-tests/wpt that referenced this issue Mar 7, 2022
This:

 * Deals with changes in how mypy handles metaclasses

 * Prefers sys.platform == "win32" due to
   python/mypy#8166 and mypy not having
   WindowsError defined by default any more

 * Installs various typestubs

 * Rewrites tox.ini to avoid duplicating everything, and allowing new
   versions of Python to be easily tested (as tox -e py310-mypy will
   now work without further changes).
foolip added a commit to web-platform-tests/wpt that referenced this issue Mar 7, 2022
Changes to adapt:

 * Deals with changes in how mypy handles metaclasses

 * Prefers sys.platform == "win32" due to
   python/mypy#8166 and mypy not having
   WindowsError defined by default any more

 * Installs various typestubs

 * Rewrites tox.ini to avoid duplicating everything, and allowing new
   versions of Python to be easily tested (as tox -e py310-mypy will
   now work without further changes).

 * Make mypy warn when it thinks code is unreachable.

Co-authored-by: Sam Sneddon <gsnedders@apple.com>
Co-authored-by: Philip Jägenstedt <philip@foolip.org>
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 22, 2022
… a=testonly

Automatic update from web-platform-tests
Bump mypy from 0.812 to 0.931 in /tools (#33082)

Changes to adapt:

 * Deals with changes in how mypy handles metaclasses

 * Prefers sys.platform == "win32" due to
   python/mypy#8166 and mypy not having
   WindowsError defined by default any more

 * Installs various typestubs

 * Rewrites tox.ini to avoid duplicating everything, and allowing new
   versions of Python to be easily tested (as tox -e py310-mypy will
   now work without further changes).

 * Make mypy warn when it thinks code is unreachable.

Co-authored-by: Sam Sneddon <gsnedders@apple.com>
Co-authored-by: Philip Jägenstedt <philip@foolip.org>
--

wpt-commits: 51ad530bef4a8396c5706f508e46f256c21b2a57
wpt-pr: 33082
aosmond pushed a commit to aosmond/gecko that referenced this issue Mar 26, 2022
… a=testonly

Automatic update from web-platform-tests
Bump mypy from 0.812 to 0.931 in /tools (#33082)

Changes to adapt:

 * Deals with changes in how mypy handles metaclasses

 * Prefers sys.platform == "win32" due to
   python/mypy#8166 and mypy not having
   WindowsError defined by default any more

 * Installs various typestubs

 * Rewrites tox.ini to avoid duplicating everything, and allowing new
   versions of Python to be easily tested (as tox -e py310-mypy will
   now work without further changes).

 * Make mypy warn when it thinks code is unreachable.

Co-authored-by: Sam Sneddon <gsnedders@apple.com>
Co-authored-by: Philip Jägenstedt <philip@foolip.org>
--

wpt-commits: 51ad530bef4a8396c5706f508e46f256c21b2a57
wpt-pr: 33082
@AlexWaygood AlexWaygood added the topic-runtime-semantics mypy doesn't model runtime semantics correctly label Mar 29, 2022
@JukkaL
Copy link
Collaborator

JukkaL commented Apr 23, 2023

#10686 has most of the implementation, but tests are failing and the PR is a bit stale. Any help with finishing up the PR would be appreciated!

@Avasam
Copy link
Sponsor Contributor

Avasam commented Oct 22, 2023

I'd also like to note that the platform.system() for is used and recommended in https://peps.python.org/pep-0508/ and https://peps.python.org/pep-0722/.
And is what setuptools currently uses pypa/setuptools#3979 (comment)

@brettcannon brettcannon changed the title Recognize platform.system() as well as sys.platform is Recognize platform.system() as well as sys.platform Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants