-
Notifications
You must be signed in to change notification settings - Fork 287
Update specification for directives for sys.implementation and sys.platform checks. #2173
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -145,8 +145,13 @@ left undefined by the typing spec at this time. | |
| Version and platform checking | ||
| ----------------------------- | ||
|
|
||
| Type checkers are expected to understand simple version and platform | ||
| checks, e.g.:: | ||
|
|
||
| Type checkers should support narrowing based on ``sys.version_info``, ``sys.platform``, and ``sys.implementation.name`` checks. | ||
|
|
||
| sys.version_info checks | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers should support ``sys.version_info`` comparisons:: | ||
|
|
||
| import sys | ||
|
|
||
|
|
@@ -155,12 +160,65 @@ checks, e.g.:: | |
| else: | ||
| # Python 3.11 and lower | ||
|
|
||
| **Supported patterns:** | ||
|
|
||
| * Equality: ``sys.version_info == (3, 10)`` | ||
| * Inequality: ``sys.version_info != (3, 9)`` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think inequality checks should also not be supported, for the same reason discussed above for equality checks. |
||
| * Comparison: ``sys.version_info >= (3, 10)``, ``sys.version_info < (3, 12)`` | ||
| * Tuple slicing: ``sys.version_info[:2] >= (3, 10)`` | ||
| * Element access: ``sys.version_info[0] >= 3`` | ||
|
Comment on lines
+168
to
+169
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for these cases (and even for the basic comparison case) it's also important to specify which elements of the tuple are supported. For example, are type checkers expected to support I think we should be explicit that type checkers should only be expected to have special handling for the first two tuple elements (major and minor version); anything more than that adds a lot of complexity to type checker configuration for little to no gain.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I think that is a useful restriction.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I think that is a useful restriction.
Comment on lines
+168
to
+169
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the named attributes? Should type checkers support e.g. (I don't think this is useful enough to require, but maybe we should be explicit that support for it is not expected.)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From a MicroPyton perspective I concur, as it does not support named attributes for this, but I don't necessarily want to force that on other implementations.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From a MicroPyton perspective I concur, as it does not support named attributes for this, but I don't necessarily want to force that on other implementations.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From a MicroPyton perspective I concur, as it does not support named attributes for this, but I don't necessarily want to force that on other implementations. |
||
|
|
||
| sys.platform checks | ||
| ^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers should support ``sys.platform`` comparisons:: | ||
|
|
||
| import sys | ||
|
|
||
| if sys.platform == 'win32': | ||
| # Windows specific definitions | ||
| else: | ||
| # Posix specific definitions | ||
|
|
||
| Don't expect a checker to understand obfuscations like | ||
| if sys.platform in ("linux", "darwin"): | ||
| # Platform-specific stubs for Linux and macOS | ||
| ... | ||
|
|
||
| **Supported patterns:** | ||
|
|
||
| * Equality: ``sys.platform == "linux"`` | ||
| * Inequality: ``sys.platform != "win32"`` | ||
| * Membership: ``sys.platform in ("linux", "darwin")`` | ||
| * Negative membership: ``sys.platform not in ("win32", "cygwin")`` | ||
|
|
||
| sys.implementation.name checks | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think adding this has does have significant ecosystem costs (as outlined by @erictraut in https://discuss.python.org/t/proposal-to-improve-support-for-other-python-platforms-in-the-typing-specification/91877/3 ). In the end I think it is probably worth it, because without it, type-checking for alternative implementations of Python seems quite difficult. The cost to type checkers themselves seems relatively low: one new config option, defaulting to
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ecosystem is larger than just one implementation.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ecosystem is larger than just one implementation. |
||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers should support ``sys.implementation.name`` comparisons:: | ||
|
|
||
| import sys | ||
| if sys.implementation.name == "cpython": | ||
| # CPython-specific stub | ||
| if sys.implementation.name == "micropython": | ||
| # MicroPython-specific stub | ||
|
|
||
| **Supported patterns:** | ||
|
|
||
| * Equality: ``sys.implementation.name == "cpython"`` | ||
| * Inequality: ``sys.implementation.name != "cpython"`` | ||
| * Membership: ``sys.implementation.name in ("pypy", "graalpy")`` | ||
| * Negative membership: ``sys.implementation.name not in ("cpython", "pypy")`` | ||
|
|
||
| Common values: ``"cpython"``, ``"pypy"``, ``"micropython"``, ``"graalpy"``, ``"jython"``, ``"ironpython"`` | ||
|
|
||
| Configuration | ||
| ^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers should provide configuration to specify target version, platform, and implementation. The exact mechanism is implementation-defined. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Target version to what granularity?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point, I think; as you suggested; Major.Minor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point, I think; as you suggested; Major.Minor |
||
|
|
||
| Complex Expressions | ||
| ^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers are not required to evaluate complex expressions involving these variables. | ||
| Therefore do not expect a checker to understand obfuscations like: | ||
| ``"".join(reversed(sys.platform)) == "xunil"``. | ||
|
|
||
| .. _`deprecated`: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example doesn't make much sense, since it would never be true at runtime on any actual Python version.
A realistic example that might actually return
Truewould need to look likesys.version_info == (3, 13, 1, "final", 0)-- but how useful is that in practice?I would suggest not specifying that equality checks should be supported at all; I don't think they are usable for any realistic scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the specific version should be adjusted, but I don't think that testing for equality should be dismissed, or truncated to an arbitrary number of nodes.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what the possible use case is here. Can you elaborate?
To be clear, at runtime equality checks with
sys.version_infowill never returnTrueunless you provide all five elements of thesys.version_infotuple. Every other equality check will always returnFalse. So this is only usable if you check all five elements, and making that useful requires that type checkers allow specifying the Python version down to "releaselevel" and "serial" level in their configurations.From a type checker implementer perspective, I feel pretty strongly that we would never implement support for this, because it is a bunch of work and it is not useful in any realistic case. I see this as a blocker for this proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the clear feedback,
Do I understand correctly that Major.Minor would be acceptable for comparison?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that all
sys.version_infochecks should only handle major/minor. That implies that support for inequality/equality checks against all ofsys.version_infomust be removed from the proposal, because there is no way to perform a useful equality/inequality check against all ofsys.version_info(as shown in this example) that only considers major/minor.I'm fine with supporting
sys.version_info[:2] == (3, 14)! So if we are explicit that equality/inequality checks are supported against only the first two elements, but not against the entire tuple, that resolves my objection.