Conversation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 85.05% to 85.42%. The percentage of expected errors that received a diagnostic increased from 78.05% to 78.15%. The number of fully passing files held steady at 63/132. SummaryHow are test cases classified?Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (
Test file breakdown1 file altered
True positives added (1)1 diagnostic
False positives removed (4)4 diagnostics
True positives changed (4)4 diagnostics
False positives changed (1)1 diagnostic
|
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 8 | 0 |
invalid-assignment |
3 | 2 | 0 |
unresolved-attribute |
0 | 0 | 1 |
| Total | 3 | 10 | 1 |
deb37bc to
4790e66
Compare
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
547d926 to
5c43bb1
Compare
sharkdp
commented
Feb 27, 2026
| // For dataclass fields with converters, the write type is the converter's | ||
| // input type, not the field's declared type. | ||
| let effective_write_type = |attr_ty: Type<'db>| -> Type<'db> { | ||
| if let Type::NominalInstance(instance) = object_ty { |
Contributor
Author
There was a problem hiding this comment.
Note that we desconstruct unions / intersections at a higher level, so we can narrowly match on NominalInstance here and still validate writes to unions of field types with converters involved.
Contributor
There was a problem hiding this comment.
We might need a .resolve_type_alias(db)? (And a test for writing to something typed as an alias to a dataclass instance type).
5c43bb1 to
91bdb3e
Compare
carljm
approved these changes
Mar 6, 2026
crates/ty_python_semantic/resources/mdtest/dataclasses/dataclass_transform.md
Show resolved
Hide resolved
|
|
||
| @my_model | ||
| class WithClassConverter: | ||
| a: int = field(converter=PermissiveNumber) |
Contributor
There was a problem hiding this comment.
I'm confused by the int annotation here. PermissiveNumber is a callable which accepts an int | str and returns a PermissiveNumber instance. So I would expect a: PermissiveNumber here, which seems to match the conformance suite expectations: https://github.com/python/typing/blob/main/conformance/tests/dataclasses_transform_converter.py#L102
It seems like this line as written should be an error, and it is an error in pyright/pyrefly/mypy.
a: str is also not an error here in ty -- it seems like we aren't validating the type of class converter fields at all?
Contributor
Author
There was a problem hiding this comment.
So I would expect
a: PermissiveNumberhere
Yes, absolutely. I think I rewrote the test too often and then overlooked this.
it seems like we aren't validating the type of class converter fields at all?
Yes, thanks for catching this! This is an artifact of the fact that we treat field(…) return types in a special way, because they're often annotated incorrectly. Looking into it.
| if !matches!(field_policy, CodeGeneratorKind::DataclassLike(_)) { | ||
| return None; | ||
| } | ||
| let fields = self.own_fields(db, None, field_policy); |
Contributor
There was a problem hiding this comment.
Does this do the right thing for writes to a subclass, where the converter-using field is inherited from a base class?
| } | ||
|
|
||
| if let Some(converter_ty) = converter_input_type { | ||
| field_ty = converter_ty; |
Contributor
There was a problem hiding this comment.
I think this method is currently also used for synthesizing __replace__ signature, but I think __replace__ should always use the raw field type, not the converter input type.
| // For dataclass fields with converters, the write type is the converter's | ||
| // input type, not the field's declared type. | ||
| let effective_write_type = |attr_ty: Type<'db>| -> Type<'db> { | ||
| if let Type::NominalInstance(instance) = object_ty { |
Contributor
There was a problem hiding this comment.
We might need a .resolve_type_alias(db)? (And a test for writing to something typed as an alias to a dataclass instance type).
Comment on lines
+1145
to
+1147
| let mut input_types = UnionBuilder::new(db); | ||
| let mut found_any = false; | ||
| for binding in converter_ty.bindings(db).iter_flat() { |
Contributor
There was a problem hiding this comment.
The use of iter_flat means that we collapse the union/intersection structure of converter_ty.
If it is an intersection of callables, I think unioning the discovered first-parameter types is actually correct (an intersection of callables is similar to overloads, in that the callable accepts all of those types.)
But if converter_ty is a union of callables, then I think technically we should build an intersection of their first parameter types? If the converter either accepts A or B, but we don't know which, then only A & B is safe to provide to the field.
(Totally open to saying we don't need to care about this, but it's at least worth a comment, I think. Pyright and mypy seem to just fail in this union-of-callables case and ignore the converter entirely; pyrefly does the same as this PR and uses the union of the first-argument types.)
91bdb3e to
1053d14
Compare
1053d14 to
c4a9297
Compare
|
As there's progress here, I thought to cross-post astral-sh/ty#1327 (comment):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for dataclass field
converters.closes astral-sh/ty#972
Ecosystem impact
Lots of removed false positives on attrs, home-assistant/core, and trio.
Typing conformance results
With this PR, we pass almost all tests in
dataclasses_transform_converter.py. The remaining problem in this test suite is not related to dataclasses or dataclass converters, but rather to a limitation in our generics solver (we don't understand the call tofield, and therefore don't recognize the converter function).Test Plan
New Markdown tests