Conversation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 85.93% to 86.29%. The percentage of expected errors that received a diagnostic increased from 80.02% to 80.11%. The number of fully passing files held steady at 66/133. 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
|
This comment was marked as outdated.
This comment was marked as outdated.
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
0 | 40 | 0 |
invalid-argument-type |
0 | 8 | 0 |
invalid-assignment |
0 | 2 | 0 |
invalid-return-type |
0 | 1 | 0 |
unresolved-attribute |
0 | 0 | 1 |
| Total | 0 | 51 | 1 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
Raw diff:
attrs (https://github.com/python-attrs/attrs)
- tests/dataclass_transform_example.py:23:17 error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal[b"42"]`
- tests/test_hooks.py:70:15 error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal["3"]`
- tests/test_next_gen.py:388:9 error[invalid-assignment] Object of type `Literal["11"]` is not assignable to attribute `x` of type `int`
- tests/test_next_gen.py:394:13 error[invalid-assignment] Object of type `Literal["9"]` is not assignable to attribute `x` of type `int`
- tests/test_next_gen.py:380:14 error[unresolved-attribute] Object of type `dataclasses.Field` has no attribute `validator`
+ tests/test_next_gen.py:380:14 error[unresolved-attribute] Object of type `dataclasses.Field[int]` has no attribute `validator`
- typing-examples/mypy.py:181:13 error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal["on"]`
- typing-examples/mypy.py:182:13 error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal["yes"]`
- typing-examples/mypy.py:185:13 error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal["n"]`
core (https://github.com/home-assistant/core)
- homeassistant/helpers/entity_registry.py:1458:13 error[invalid-argument-type] Argument is incorrect: Expected `str`, found `None | str`
- homeassistant/helpers/entity_registry.py:1462:13 error[invalid-argument-type] Argument is incorrect: Expected `ReadOnlyEntityOptionsType`, found `Mapping[str, Mapping[str, Any]] | None`
trio (https://github.com/python-trio/trio)
- src/trio/_tests/test_highlevel_open_tcp_listeners.py:235:35 error[invalid-argument-type] Argument is incorrect: Expected `SocketKind`, found `int`
sharkdp
force-pushed
the
david/converter
branch
from
deb37bc to
4790e66
Compare
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
sharkdp
force-pushed
the
david/converter
branch
2 times, most recently
from
547d926 to
5c43bb1
Compare
sharkdp
force-pushed
the
david/converter
branch
from
5c43bb1 to
91bdb3e
Compare
sharkdp
requested review from
AlexWaygood,
carljm,
dcreager and
ibraheemdev
as code owners
| } | ||
|
|
||
| if let Some(converter_ty) = converter_input_type { | ||
| field_ty = converter_ty; |
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.
There was a problem hiding this comment.
I think
__replace__should always use the raw field type, not the converter input type.
Hm, that would depend on the actual __replace__ implementation, no? I only know one library which makes use of converter (attrs), and it seems that they do call the converter function when using replace(…):
- https://github.com/python-attrs/attrs/blob/25b74d6a4274d938b1ef15a4df658ad5d2c92310/src/attr/_make.py#L1107
- https://github.com/python-attrs/attrs/blob/25b74d6a4274d938b1ef15a4df658ad5d2c92310/src/attr/_make.py#L642 (calls the constructor with all previous fields and all changes, and the constructor calls the converter)
I also verified this behavior locally with a small example.
I added a test to reflect and describe the current behavior.
There was a problem hiding this comment.
Good call! FWIW it looks like pyrefly and pyright also agree with the behavior you implemented here; mypy does not. (But I guess mypy has an attrs plugin, so probably doesn't care.)
| let mut input_types = UnionBuilder::new(db); | ||
| let mut found_any = false; | ||
| for binding in converter_ty.bindings(db).iter_flat() { |
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.)
There was a problem hiding this comment.
I decided to only add a comment why this is currently not supported.
But this raises an interesting question: should a union of Callables be assignable to a Callable[[S], T] where S and T are type variables? We currently don't allow this:
from typing import Callable
def identity[S, T](c: Callable[[S], T]) -> Callable[[S], T]:
return c
class A: ...
class B: ...
def _(c: Callable[[A], A] | Callable[[B], B]):
reveal_type(identity(c)) # should this be Callable[[A & B], A & B]?There was a problem hiding this comment.
This is a fun one to run through multiplay :) One of those "every type checker has a different answer" cases. Won't summarize, but gist 252ff444d987ffdc24bfa79f13909615 if you're curious.
I think the comment here is fine.
sharkdp
force-pushed
the
david/converter
branch
2 times, most recently
from
1053d14 to
c4a9297
Compare
|
As there's progress here, I thought to cross-post astral-sh/ty#1327 (comment):
|
sharkdp
force-pushed
the
david/converter
branch
from
c4a9297 to
e640411
Compare
|
There were some additional complications here regarding generic converter functions that showed up in the ecosystem. No other type checker can handle this situation either, and so I opted for a simple strategy for now to avoid false positives (default-specialize the callable to make it assignable). Once our generic solver can properly handle passing generic callables to generic callables, we can revisit this, but it seems relatively complex, and at the same time not very important (but I added a TODO). |
* main: [ty] make `test-case` a dev-dependency (#24187) [ty] implement cycle normalization for more types to prevent too-many-cycle panics (#24061) [ty] Silence all diagnostics in unreachable code (#24179) [ty] Intern `InferableTypeVars` (#24161) Implement unnecessary-if (RUF050) (#24114) Recognize `Self` annotation and `self` assignment in SLF001 (#24144) Bump the npm version before publish (#24178) [ty] Disallow Self in metaclass and static methods (#23231) Use trusted publishing for NPM packages (#24171) [ty] Respect non-explicitly defined dataclass params (#24170) Add RUF072: warn when using operator on an f-string (#24162) [ty] Check return type of generator functions (#24026) Implement useless-finally (RUF-072) (#24165) [ty] Add test for a dataclass with a default field converter (#24169) [ty] Dataclass field converters (#23088) [flake8-bandit] Treat sys.executable as trusted input in S603 (#24106) [ty] Add support for `typing.Concatenate` (#23689) `ASYNC115`: autofix to use full qualified `anyio.lowlevel` import (#24166) [ty] Disallow read-only fields in TypedDict updates (#24128) Speed up diagnostic rendering (#24146)
3 participants
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