typing icon indicating copy to clipboard operation
typing copied to clipboard

Spec: Version and platform checking underspecified

Open srittau opened this issue 1 year ago • 9 comments

Currently, the "Version and platform checking" section of the typing spec is very barebones. It basically says:

Type checkers are expected to understand simple version and platform checks. [...] Don’t expect a checker to understand obfuscations like "".join(reversed(sys.platform)) == "xunil".

Of course, there's a world of difference between these two forms.

The type stubs document goes into a bit more of detail, and reflects the currently implemented functionality quite well, as far as I know. It would be a good starting point.

Maybe it would also make sense to support platform checks using startswith(), as that is officially recommended in the Python docs, and is required to properly support some Unix variants, like FreeBSD, that include the version number in the platform string. See also python/typeshed#11876.

srittau avatar May 08 '24 10:05 srittau

I'm in favor of making the language in the typing spec more precise here, but I'd prefer not to expand the set of supported expression forms here unless there's a really compelling use case identified that cannot be handled with the currently-supported expression forms. A single example doesn't constitute a compelling use case, IMO.

For the record, pyright supports:

  • sys.platform <equality_operator> LS
  • sys.version_info[0] >= LI
  • sys.version_info <comparison_operator> <version_tuple>
  • os.name <equality_operator> LS
  • Combinations of the above using not, and, and or

Where LI means "literal integerandLS` means "literal string"

<version_tuple> is one of (LI1, ), (LI1, LI2), (LI1, LI2, LI3), (LI1, LI2, LI3, LS4), or (LI1, LI2, LI3, LS4, LI5)

<equality_operator> is one of == or !=

<comparison_operator> is one of <, <=, ==, !=, >, or >=

The above list was established based on real-world use cases that I've run across over the years.

I don't think mypy currently supports os.name or sys.platform, but someone should confirm.

erictraut avatar May 08 '24 17:05 erictraut

Mypy supports sys.platform but not os.name. It also supports sys.version_info[0] (and possibly more variants; I haven't checked the mypy code). Example: https://mypy-play.net/?mypy=latest&python=3.10&gist=cbd11992b8631d8bbf8d217de99386e2.

JelleZijlstra avatar May 08 '24 17:05 JelleZijlstra

I'm in favor of making the language in the typing spec more precise here, but I'd prefer not to expand the set of supported expression forms here unless there's a really compelling use case identified that cannot be handled with the currently-supported expression forms.

If I remember correctly, we have several cases in the stdlib stubs where BSD constants are not correctly guarded. While the BSD platforms are certainly not the most important ones, I think the typing system should support all supported Python platforms properly. Especially, since the recommended form to use sys.platform currently doesn't work with typing, i.e. users following the docs won't have code that's properly type checked.

srittau avatar May 08 '24 17:05 srittau

mypy has actually always supported sys.platform.startswith, since 2016 (in addition to == and !=) mypy doesn't support os.name mypy supports comparisons against a potentially indexed or sliced sys.version_info

Details:

  • https://github.com/python/mypy/blob/master/mypy/reachability.py#L218
  • https://github.com/python/mypy/blob/master/mypy/reachability.py#L291

hauntsaninja avatar May 08 '24 20:05 hauntsaninja

I'm in favor of making the language in the typing spec more precise here, but I'd prefer not to expand the set of supported expression forms here unless there's a really compelling use case identified that cannot be handled with the currently-supported expression forms.

If I remember correctly, we have several cases in the stdlib stubs where BSD constants are not correctly guarded. While the BSD platforms are certainly not the most important ones, I think the typing system should support all supported Python platforms properly. Especially, since the recommended form to use sys.platform currently doesn't work with typing, i.e. users following the docs won't have code that's properly type checked.

Yeah, there seems to be quite a few instances of these platform checks where correctness is lacking due to limited support in the typing spec:

  • https://github.com/python/typeshed/blob/main/stdlib/_socket.pyi#L180
  • https://github.com/python/typeshed/blob/main/stdlib/_socket.pyi#L414
  • https://github.com/python/typeshed/blob/main/stdlib/_socket.pyi#L533
  • https://github.com/python/typeshed/blob/main/stdlib/_socket.pyi#L541
  • https://github.com/python/typeshed/blob/main/stdlib/mmap.pyi#L103
  • https://github.com/python/typeshed/blob/main/stdlib/time.pyi#L15
  • https://github.com/python/typeshed/blob/main/stdlib/time.pyi#L26
  • https://github.com/python/typeshed/blob/main/stdlib/os/init.pyi#L87
  • https://github.com/python/typeshed/blob/main/stdlib/os/init.pyi#L710

This also isn't a complete list as this was from just a very brief search for "FreeBSD" (plus the Solaris only one I just happened to find along the way). But to @erictraut's point, it's still a somewhat niche use case and I'm not sure if this warrants breaking existing tools such as Pyright.

I'm curious if any actual non-Windows/Linux/Mac users have actually brought up issues related to this as that usually would imply that likely many more have encountered such issues, as well. For the most part, the current platform checks might be good enough, but there were some that basically had to be incorrect due to the limitations in the current spec:

if sys.version_info >= (3, 9) and sys.platform == "linux":
    # Availability: Linux >= 2.6.20, FreeBSD >= 10.1

bytemarx avatar May 09 '24 01:05 bytemarx

I agree we should precisely specify the set of checks that type checkers are required to support. This allows library authors to be confident that the code they write will be understood consistently by type checkers. However, type checkers should be allowed to support additional version or platform checks if they choose to do so, and projects like typeshed may choose to apply more restrictive policies than the typing spec.

As I see it, there are two main motivations for adding support for some pattern:

  • Without the pattern, some important pattern cannot be expressed in the type system. For example, if we didn't have platform checks at all, type checkers couldn't use typeshed stubs to warn users if they're using functions that don't exist on their platform. In the terms of PEP 729, this is about making the type system useful.
  • The pattern appears commonly in real-world code. For example, supporting if sys.version_info[0] >= 3 isn't strictly necessary (you could write if sys.version_info >= (3,) instead), but this pattern appears in a lot of existing code, so adding support for this pattern eases adoption of typing. In PEP 729's terms, this is about making the type system usable.

Here's some thoughts on the specific checks that should be allowed:

  • It makes sense to look at the platforms supported by CPython, as @srittau suggested above. Those are defined in https://peps.python.org/pep-0011/. I think we should aim to make it possible to write typed code on all supported platforms up to Tier 3, though obviously the higher the tier, the higher the priority. This would imply support for various flavors of Linux, MacOS, Windows, WASM, iOS, and FreeBSD.
  • sys.platform.startswith probably makes sense to add since it helps FreeBSD (a Tier 3 platform), is already supported by mypy, and doesn't seem difficult to add to other type checkers.
  • Mypy has open issues asking for platform.system() (https://github.com/python/mypy/issues/8166) and os.name (https://github.com/python/mypy/issues/13002) but they're not especially popular. Adding support for these would help some real-world code but I'm not sure they're important enough to add to the spec.
  • For sys.version_info checks I'd start with the intersection of the checks supported by mypy and pyright (as listed by @erictraut and @hauntsaninja above). If there are other commonly useful checks that aren't supported by both, we can add them.

JelleZijlstra avatar May 09 '24 02:05 JelleZijlstra

There's also https://github.com/python/mypy/issues/9025 requesting that the assertion form work on any code path, not just at the top level (as a workaround, if <sys.platform check>: assert False already works for that purpose).

Where that feature helps is as a way of notifying typecheckers of genuinely project specific ways of checking for platform support.

I'm not sure if it's feasible, but it would also be nice if platform dependent attribute fallbacks didn't fail typechecking. Currently the attribute error needs to be ignored, and then the unused ignore needs to be ignored on the platforms that do provide the attribute:

try:
    fs_sync = os.sync # type: ignore[attr-defined,unused-ignore]
except AttributeError:
    # No os.sync on Windows
    def fs_sync() -> None:
        pass

ncoghlan avatar Aug 27 '24 07:08 ncoghlan

Regrettably, with mypy 1.11.2 (compiled: yes) and CPython 3.12.6, the OS-dependent type checking is really hit-or-miss.

I suggest to increase efficiency, the set of supported platforms for a certain Python package should be configurable in the Mypy configuration, rather than always checking for all possible cases dynamically. Then, type check the whole code base for every supported platform and eliminate those type checking faults that do not appear under every supported platform, where all such faults occur within at most one program's code base and also under at most one type of platform*.

*: Please make use of the property that platform is a whole program invariant (for lack of a better term). Of course, that's still troublesome if Mypy can't distinguish whole libraries or programs from one another in a package that bundles multiple. I think it's reasonable if Mypy requires the programmer to specify the ‘units of execution’ (a program in a strict sense), e.g. those that are imported in a single interpreter instance or have a single virtual address space. Shouldn't be too difficult if the package is reasonably structured (e.g., the static structure, i.e., source code, is a structural analog of the dynamic structure, i.e., run time components).

Works

import sys
from platform import system

if sys.platform.startswith("linux") and system() == "Linux":
    from os import sched_getaffinity
if sys.platform.startswith("linux"):
    cpucount = len(sched_getaffinity(0))
else:
    cpucount = cpu_count()

Doesn't work

if system() == "Linux" and sys.platform.startswith("linux"):
    from os import sched_getaffinity
from sys import platform as sys_platform
from platform import system

if sys_platform.startswith("linux") and system() == "Linux":
    from os import sched_getaffinity
cpucount = len(sched_getaffinity(0)) if sys.platform.startswith("linux") else cpu_count()

sanmai-NL avatar Oct 01 '24 09:10 sanmai-NL