Repository navigation
Conversation
4151e84 to
d7fbb9e
Compare
d7fbb9e to
62d90dc
Compare
|
I have updated the text to better clarify the exact comparisons for each of the supported attributes, |
62d90dc to
ff338e3
Compare
|
Dear TC, If so kindly let me know how to proceed. |
JelleZijlstra
left a comment
There was a problem hiding this comment.
I'm generally supportive but I think this change needs some more editing, and I'd like support in at least some type checkers (at least an open PR) before we start mandating this pattern.
c575c15 to
c951b00
Compare
|
Dear typing council, However now it appears that this PR is in a Catch-22 situation; Lacking response I plan close this in a few weeks |
This is not the case at all. It is very typical for type checkers to implement features even if they are not in the spec. It is very helpful to have feedback from type checker authors to be able to understand potential problems with spec changes. I think this has worked quite well in the past.and if a feature is useful enough there is usually a type checker that will implement it. |
carljm
left a comment
There was a problem hiding this comment.
Thanks for your patience, @Josverl. I'm not sure we've fully reached consensus yet on what should be supported here, but I also generally support making this clearly specified. I also think this still needs some editing to give a clear and unambiguous presentation of the requirements.
|
Thanks all for the renewed attention and comments.
|
b3b76c1 to
98e3697
Compare
|
Again thanks for the feedback and suggestions to improve the spec. I processed the comments and suggestion - and commented or marked them as resolved to keep track. Open topics. 1. 2-Tuple or 3-Tuple version comparisons.
2. Should 3. Scope I think the commits should be squashed when merging - I've kept them separate for now. |
|
We could eliminate open-question (2) and at least partially address (3) by just considering containment checks as out of scope to specify for now. Considering they don't add any new capability, just allow writing more concise code, they aren't really necessary. We could leave this out of the spec / conformance suite for now and up to individual type checkers. Similarly maybe for |
+1
Unlike containment checks, |
That would be disappointing, especially as one of the first responses in May'25 in DPO included :
But reality is that there different opinions based on different considerations, and
|
|
Not closed in anger - just a typo |
Inline list and set membership tests now preserve the values of their
individual elements, allowing ty to determine when a condition is always
true or false. With the target platform set to Linux, both `sys.platform
in ["linux", "windows"]` and `sys.platform in {"linux", "windows"}`
select only the matching branch, so an assignment there is inferred as
`Literal[1]`.
Reuse the precise element extraction used for iteration and membership
narrowing without changing the inferred container type. Lists and sets
use the existing tuple membership equality evaluator, including its
identity-or-equality behavior. For sets, assume equality is an
equivalence relation and equal objects have equal hashes. Checking
reflexive equality across union alternatives also improves existing
tuple membership inference.
Context is that in python/typing#2173 people are
wanting to specify that `sys.platform in ["linux", "windows"]` should
work. This makes it work in ty so that we can be fine with any outcome
of that specification change.
## Test plan
Mdtests cover platform-dependent branch reachability; positive and
negative membership in inline lists and sets; inferred literal values
and unions; fixed-length unpacking; stored mutable containers; custom
equality and identity; comparison chains; and preservation of enum-set
narrowing.
## Ecosystem report
The comparison for head `90111f10da9359722abb903351b89aaae0f9bcba` adds
four `redundant-condition-strict` warnings. The paasta and SciPy
warnings correctly identify disjoint membership checks. The dd-trace-py
warning exposes existing limitations in modeling dynamic `setattr`; the
Scrapy warning exposes existing attribute narrowing across method calls
and `await`. Equivalent equality checks already exhibit both limitations
on the base revision. These existing limitations are accepted for this
change. See the [full ecosystem
report](#28676 (comment))
for minimized examples and exact comparison details.
|
Just to avoid unneeded waiting: |
|
@carljm, CI also seems to have forgotten that I did sign the CLA with all my identities, re-signing is not possible either. |
carljm
left a comment
There was a problem hiding this comment.
I think the CLA signing failure is because your latest commit (unlike the previous ones) is authored by Jos_Verlinde@hotmail.com, which GitHub can't associate with your GitHub account. If you can amend that commit with a recognized email address and force push, I suspect the CLA signing check will pass.
Barring my two nits below, I think this PR is ready for conformance tests. @jorenham @davidhalter @JelleZijlstra @rchen152 any objections to the spec updates here, before @Josverl writes conformance tests?
|
I'll try to take a look this evening |
jorenham
left a comment
There was a problem hiding this comment.
Looks good overall, thanks. I left some comments for some minor issues / nits / suggestions.
| Required forms and optional extensions | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers must support the forms specified above as required. Support for additional |
There was a problem hiding this comment.
consistency nit
| Type checkers must support the forms specified above as required. Support for additional | |
| Type checkers should support the forms specified above as required. Support for additional |
There was a problem hiding this comment.
I do not think that "should" can be combined with "required" in this sentence.
That would degrade the meaning of the "required" statements in the prior paragraphs, which I think is incorrect.
There was a problem hiding this comment.
IMO you should just drop the sentence. The spec is clear in the "above" cases where a type checker has to support it and where it's optional, so the sentence feels unnecessary.
There was a problem hiding this comment.
Dropped that sentence and reduced the section heading to: Optional extensions
bc8965e to
d2c4133
Compare
d2c4133 to
e612515
Compare
|
@carljm, @jorenham Also have tried to make a start on the conformance tests in #2375. CLA signature appears to have been caused by some parts of the CLA check assuming that smtp e-mail addresses are case sensitive, but that was solved simply. |
|
@Josverl Why do we need to do conformance tests in a separate PR? I would find it much easier to review, approve, and merge the spec changes and conformance tests together, in the same PR. Conformance tests may reveal changes that we need to make to the spec, and we need to ensure they are consistent with each other. |
The original plan I was set out on was much simpler, and I explicitly started out with: "This is my first post to the Python discuss, so please be gentle." And was guided by the TC decision process that documents:
This is my first PR to "the big" Python, it is open more than 10 months with over 150 comments,
to be very clear, I could not have done this without all your help, but it has taken over 17 months since first discussion. ( with time-gaps both from all involved) On the conformance tests I replied earlier :
So what are the rules for creating conformance test , for functionality that is not currently supported ? Note that the TC process document does not mention conformance test.
To extend beyond that, is perhaps apparent you. To me it is not. Yes, I can and have set an AI to work, and revised the results to avoid the most apparent mistakes. What is the process after that ? So that is why I split them , because I would not want to loose this part, and currently I am unable to commit to finish something that I cannot fully understand the implications of. I would welcome a conversation on the challenges and opportunities involved in a more private setting. |
|
@Josverl are you on Discord? I'd be happy to connect there and talk through issues in a more informal way. (I can't speak for Carl but he's also on Discord and might also be willing to chat there.) |
Yes, thanks for the offer. I shared my details via E-mail. |
|
Yes, happy to discuss in Discord! To answer a few specific questions/comments:
|
davidhalter
left a comment
There was a problem hiding this comment.
First of all, thank you @Josverl for staying on this for such a long time. This is one of the most discussed issues and it's easy to give up at some point. Also thank you to Carl for doing most of the review work.
The PR is in a very good shape IMO. I have a few questions that are directed to other people than @Josverl, and a small suggestion to drop a sentence, otherwise no work should be necessary.
I would definitely not reduce the scope for now. While I would be grateful for conformance tests here I'd rather merge it as is and do the conformance tests later if @Josverl decides to not work on this anymore, since I think it's in a really good state and a massive improvement from the previous version. Though if you @Josverl still have some energy left in the tank, conformance tests would of course be awesome. :)
Thanks all!
| ^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers may optionally support comparisons of ``os.name`` with string | ||
| literals. |
There was a problem hiding this comment.
Why is this optional? Sorry I haven't followed the discussion for a while and I'm sure it has been discussed. It might make sense to make this mandatory as well.
There was a problem hiding this comment.
Because it's currently optional in the the conformance suite:
typing/conformance/tests/directives_version_platform.py
Lines 61 to 75 in d06c5dd
There was a problem hiding this comment.
Thanks! IMO we should probably make that mandatory then in a separate follow up PR.
| Required forms and optional extensions | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Type checkers must support the forms specified above as required. Support for additional |
There was a problem hiding this comment.
IMO you should just drop the sentence. The spec is clear in the "above" cases where a type checker has to support it and where it's optional, so the sentence feels unnecessary.
| The configuration options should allow users to specify the target values for these | ||
| symbols, so that type checkers can evaluate the version and platform checks | ||
| correctly. The exact mechanism and names for these configuration options are | ||
| implementation-specific and defined by each type checker. |
There was a problem hiding this comment.
This is more of a question to other type checker authors: Is this already implemented somewhere? I definitely like that sys.implementation.name is added, that seems like a very useful addition especially in stubs. The implementation version is probably also needed, but that asks the question: How should I infer the implementation version? Should I just default to a nullable sys.implementation.version if the implementation cpython is not used and the version is not provided?
IMO it would be nice if we added this to the conformance tests as well, but that might be a little more complicated since we would need a way to specify the implementation/version for a specific test file. So I don't think this is necessary for this PR, but I would definitely appreciate if someone of us (looking at myself as well) would do the work here.
…atform checks. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
…checks. - Introduced new TOML files for various conformance results related to directives implementation version, platform checks, and version/platform boolean checks. - Marked conformance as "Unsupported" or "Partial" based on the ability to prune branches for specific checks. - Documented unexpected errors and output related to type assignment issues, particularly with `Literal` types and incompatible types in assignments. - Updated existing TOML files to include additional error messages for unresolved references and type assertion failures. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
e612515 to
70ac79f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Stale mypy and ty overrides understate passing platform-check results, while the documentation incorrectly lists Android and iOS as Python implementations.
3 open issues
What changed in this PR
This PR expands the typing specification’s rules for version, platform, and implementation checks, helping type checkers handle platform-specific code and non-CPython implementations consistently.
Changes:
- Defines supported comparisons, Boolean combinations, optional extensions, and target configuration defaults.
- Adds conformance tests for implementation checks, platform membership and prefixes, and version boundaries.
- Records results for six type checkers and updates the conformance report and testing guidance.
| File | Description |
|---|---|
docs/spec/directives.rst |
Expands the specification and examples. |
conformance/tests/directives_version_platform.py |
Adds boundary and branch-selection tests; clarifies comments. |
conformance/tests/directives_version_platform_boolean.py |
Tests Boolean combinations of checks. |
conformance/tests/directives_platform_checks.py |
Tests platform comparisons, membership, and prefixes. |
conformance/tests/directives_implementation_version.py |
Tests CPython implementation-version defaults. |
conformance/tests/directives_implementation_name.py |
Tests implementation-name equality and membership. |
conformance/results/zuban/directives_version_platform.toml |
Updates Zuban version-check diagnostics. |
conformance/results/zuban/directives_version_platform_boolean.toml |
Records Zuban Boolean-check results. |
conformance/results/zuban/directives_platform_checks.toml |
Records Zuban platform-check results. |
conformance/results/zuban/directives_implementation_version.toml |
Records Zuban implementation-version results. |
conformance/results/zuban/directives_implementation_name.toml |
Records Zuban implementation-name results. |
conformance/results/ty/directives_version_platform.toml |
Updates ty version-check diagnostics. |
conformance/results/ty/directives_version_platform_boolean.toml |
Records ty Boolean-check results. |
conformance/results/ty/directives_platform_checks.toml |
Records ty platform-check results. |
conformance/results/ty/directives_implementation_version.toml |
Records ty implementation-version results. |
conformance/results/ty/directives_implementation_name.toml |
Records ty implementation-name results. |
conformance/results/results.html |
Adds result rows and updates summary scores. |
conformance/results/pyright/directives_version_platform.toml |
Updates Pyright version-check diagnostics. |
conformance/results/pyright/directives_version_platform_boolean.toml |
Records Pyright Boolean-check results. |
conformance/results/pyright/directives_platform_checks.toml |
Records Pyright platform-check results. |
conformance/results/pyright/directives_implementation_version.toml |
Records Pyright implementation-version results. |
conformance/results/pyright/directives_implementation_name.toml |
Records Pyright implementation-name results. |
conformance/results/pyrefly/directives_version_platform.toml |
Updates Pyrefly version-check diagnostics. |
conformance/results/pyrefly/directives_version_platform_boolean.toml |
Records Pyrefly Boolean-check results. |
conformance/results/pyrefly/directives_platform_checks.toml |
Records Pyrefly platform-check results. |
conformance/results/pyrefly/directives_implementation_version.toml |
Records Pyrefly implementation-version results. |
conformance/results/pyrefly/directives_implementation_name.toml |
Records Pyrefly implementation-name results. |
conformance/results/pycroscope/directives_version_platform.toml |
Updates Pycroscope version-check diagnostics. |
conformance/results/pycroscope/directives_version_platform_boolean.toml |
Records Pycroscope Boolean-check results. |
conformance/results/pycroscope/directives_platform_checks.toml |
Records Pycroscope platform-check results. |
conformance/results/pycroscope/directives_implementation_version.toml |
Records Pycroscope implementation-version results. |
conformance/results/pycroscope/directives_implementation_name.toml |
Records Pycroscope implementation-name results. |
conformance/results/mypy/directives_version_platform.toml |
Updates mypy version-check diagnostics. |
conformance/results/mypy/directives_version_platform_boolean.toml |
Records mypy Boolean-check results. |
conformance/results/mypy/directives_platform_checks.toml |
Records mypy platform-check results. |
conformance/results/mypy/directives_implementation_version.toml |
Records mypy implementation-version results. |
conformance/results/mypy/directives_implementation_name.toml |
Records mypy implementation-name results. |
conformance/README.md |
Documents test targets, scenarios, and coverage limitations. |
Review effort: Balanced
| conformant = "Partial" | ||
| notes = """ | ||
| Does not prune branches for tuple, set, or list membership and nonmembership checks. |
| conformant = "Partial" | ||
| notes = """ | ||
| Does not prune branches for set or list membership and nonmembership checks; tuple checks pass. |
| Default value: ``"cpython"``, unless configured otherwise. | ||
|
|
||
| Common values: ``"cpython"``, ``"pypy"``, ``"micropython"``, ``"graalpy"``, | ||
| ``"android"``, ``"ios"``, ``"ironpython"`` |
There was a problem hiding this comment.
@jorenham , IIRC these were suggested by you.
What is your take on this ?
There was a problem hiding this comment.
Ah yea sorry I intended to suggest adding ios and android to sys.platform, but when writing about the dubious jython value here I apparently ended up suggesting adding ios and android here instead.
|
I have no intent of abandoning this, I have merged the conformance tests and rebased Would still appreciate assistance wrt to the conformance tests to make sure they are correct. Unclear why RTD cannot checkout the commit. |


This PR expands and clarifies the specification for version, platform, and Python implementation checks:
sys.implementation.version.sys.platform.startswith().References :