Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.1k
Rework starargs with union argument#19651
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:master
Are you sure you want to change the base?
Rework starargs with union argument #19651
Uh oh!
There was an error while loading. Please reload this page.
Conversation
randolf-scholz commented Aug 13, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
7146a29 to 049afbcCompare This comment has been minimized.
This comment has been minimized.
randolf-scholz commented Aug 13, 2025
Hm there is still some issue to be solved with I think one could determine the correct upcast by using the solver, but I discovered some inconsistencies: #19652 |
randolf-scholz commented Aug 13, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
This really does catch quite a few false negatives, for example: Repro of defset_rgb_color(red: int, green: int, blue: int) ->None: ... _last_color_state: tuple[ str|None, int|None, tuple[int, int, int] |tuple[int|None] |None, ] color_mode, brightness, color=_last_color_stateifcolor: set_rgb_color(*color) # MASTER: no error ❌, PR: raises [arg-type] ✅Repro of fromtypingimportIterable, Hashable type _Dim=Hashable type _Dims=tuple[_Dim, ...] type _DimsLike=str|Iterable[_Dim] defpermute_dims(*dim: Iterable[_Dim]) ->None: ... deftest(dims: _DimsLike) ->None: permute_dims(*dims) # master: no error ❌, PR: raises [arg-type] ✅Repro of fromtypingimportAnyclassRequestHandler: ... type RouteContext=dict[str, Any] type URLRoutes=list[ tuple[str, type[RequestHandler]] |tuple[str, type[RequestHandler], RouteContext], ] defdemo( all_patterns: URLRoutes, extra_patterns: URLRoutes, toplevel_patterns: URLRoutes, prefix: str, data: dict[str, Any], ) ->None: forpinextra_patterns+toplevel_patterns: prefixed_pat= (prefix+p[0], *p[1:], data) all_patterns.append(prefixed_pat) # MASTER: false negative, PR: [arg-type] |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b341226 to 5e239a3Compare This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
randolf-scholz commented Aug 15, 2025
I think this is ready for initial review. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
randolf-scholz commented Aug 16, 2025
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Diff from mypy_primer, showing the effect of this PR on open source code: tornado (https://github.com/tornadoweb/tornado) + tornado/routing.py:355: error: Argument 2 to "Rule" has incompatible type "*Union[list[Any], tuple[Any], tuple[Any, dict[str, Any]], tuple[Any, dict[str, Any], str]]"; expected "Optional[dict[str, Any]]" [arg-type]+ tornado/routing.py:355: error: Argument 2 to "Rule" has incompatible type "*Union[list[Any], tuple[Any], tuple[Any, dict[str, Any]], tuple[Any, dict[str, Any], str]]"; expected "Optional[str]" [arg-type]+ tornado/routing.py:357: error: Argument 1 to "Rule" has incompatible type "*Union[list[Any], tuple[Union[str, Matcher], Any], tuple[Union[str, Matcher], Any, dict[str, Any]], tuple[Union[str, Matcher], Any, dict[str, Any], str]]"; expected "Matcher" [arg-type]+ tornado/routing.py:357: error: Argument 1 to "Rule" has incompatible type "*Union[list[Any], tuple[Union[str, Matcher], Any], tuple[Union[str, Matcher], Any, dict[str, Any]], tuple[Union[str, Matcher], Any, dict[str, Any], str]]"; expected "Optional[dict[str, Any]]" [arg-type]+ tornado/routing.py:357: error: Argument 1 to "Rule" has incompatible type "*Union[list[Any], tuple[Union[str, Matcher], Any], tuple[Union[str, Matcher], Any, dict[str, Any]], tuple[Union[str, Matcher], Any, dict[str, Any], str]]"; expected "Optional[str]" [arg-type] hydpy (https://github.com/hydpy-dev/hydpy) + hydpy/core/selectiontools.py:370: error: Argument 1 to "intersection" of "Devices" has incompatible type "*Elements"; expected "Elements" [arg-type]+ hydpy/core/pubtools.py:163: error: Argument 1 to "Timegrids" has incompatible type "*Timegrids | Timegrid | tuple[datetime | str | Date, datetime | str | Date, timedelta | str | Period]"; expected "Timegrid" [arg-type]+ hydpy/core/pubtools.py:163: error: Argument 1 to "Timegrids" has incompatible type "*Timegrids | Timegrid | tuple[datetime | str | Date, datetime | str | Date, timedelta | str | Period]"; expected "Timegrid | None" [arg-type] aiohttp (https://github.com/aio-libs/aiohttp) + aiohttp/web_urldispatcher.py:671:16: error: Exception type must be derived from BaseException (or be a tuple of exception classes) [misc] colour (https://github.com/colour-science/colour) + colour/plotting/models.py:1986: error: Argument 2 to "_linear_equation" has incompatible type "*ndarray[tuple[int], dtype[float64]]"; expected "ndarray[tuple[Any, ...], dtype[floating[_16Bit] | floating[_32Bit] | float64]]" [arg-type] core (https://github.com/home-assistant/core) + homeassistant/components/govee_light_local/light.py:216: error: Argument 2 to "set_rgb_color" of "GoveeLocalApiCoordinator" has incompatible type "*tuple[int, int, int] | tuple[int | None]"; expected "int" [arg-type]+ homeassistant/components/govee_light_local/light.py:218: error: Argument 2 to "set_temperature" of "GoveeLocalApiCoordinator" has incompatible type "*tuple[int, int, int] | tuple[int | None]"; expected "int" [arg-type] comtypes (https://github.com/enthought/comtypes) - comtypes/_memberspec.py:85: error: Unused "type: ignore" comment [unused-ignore] werkzeug (https://github.com/pallets/werkzeug) + src/werkzeug/test.py:436: error: Argument 2 to "add_file" of "FileMultiDict" has incompatible type "*Union[tuple[IO[bytes], str], tuple[IO[bytes], str, str]]"; expected "Optional[str]" [arg-type] dd-trace-py (https://github.com/DataDog/dd-trace-py) + ddtrace/llmobs/_integrations/bedrock_agents.py:237: error: Argument 1 to "set_exc_info" of "Span" has incompatible type "*tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]"; expected "type[BaseException]" [arg-type]+ ddtrace/llmobs/_integrations/bedrock_agents.py:237: error: Argument 1 to "set_exc_info" of "Span" has incompatible type "*tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]"; expected "BaseException" [arg-type]+ ddtrace/llmobs/_integrations/bedrock_agents.py:286: error: Argument 1 to "set_exc_info" of "Span" has incompatible type "*tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]"; expected "type[BaseException]" [arg-type]+ ddtrace/llmobs/_integrations/bedrock_agents.py:286: error: Argument 1 to "set_exc_info" of "Span" has incompatible type "*tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]"; expected "BaseException" [arg-type]+ ddtrace/llmobs/_experiment.py:367: error: Argument 1 to "set_exc_info" of "Span" has incompatible type "*tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]"; expected "type[BaseException]" [arg-type]+ ddtrace/llmobs/_experiment.py:367: error: Argument 1 to "set_exc_info" of "Span" has incompatible type "*tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]"; expected "BaseException" [arg-type] dedupe (https://github.com/dedupeio/dedupe) + dedupe/core.py:196: error: Argument 1 to "zip" has incompatible type "*tuple[int, Mapping[str, Any]] | tuple[str, Mapping[str, Any]]"; expected "Iterable[str]" [arg-type] materialize (https://github.com/MaterializeInc/materialize) + misc/python/materialize/mzcompose/composition.py:382: error: Argument 1 to "Popen" has incompatible type "list[object]"; expected "str | bytes | PathLike[str] | PathLike[bytes] | Sequence[str | bytes | PathLike[str] | PathLike[bytes]]" [arg-type]+ misc/python/materialize/mzcompose/composition.py:433: error: Not all union combinations were tried because there are too many unions [misc]+ misc/python/materialize/mzcompose/composition.py:434: error: Argument 1 to "run" has incompatible type "list[object]"; expected "str | bytes | PathLike[str] | PathLike[bytes] | Sequence[str | bytes | PathLike[str] | PathLike[bytes]]" [arg-type] ignite (https://github.com/pytorch/ignite) + ignite/engine/engine.py:156: error: Argument 1 to "register_events" of "Engine" has incompatible type "*type[Events]"; expected "list[str] | list[EventEnum]" [arg-type]+ ignite/contrib/engines/tbptt.py:120: error: Argument 1 to "register_events" of "Engine" has incompatible type "*type[Tbptt_Events]"; expected "list[str] | list[EventEnum]" [arg-type] xarray (https://github.com/pydata/xarray) + xarray/tests/test_namedarray.py: note: In member "test_permute_dims" of class "TestNamedArray":+ xarray/tests/test_namedarray.py:545: error: Argument 1 to "permute_dims" of "NamedArray" has incompatible type "*str | Iterable[Hashable]"; expected "Iterable[Hashable] | EllipsisType" [arg-type]+ xarray/tests/test_namedarray.py: note: In class "TestNamedArray": bokeh (https://github.com/bokeh/bokeh) + src/bokeh/server/tornado.py: note: In member "__init__" of class "BokehTornado":+ src/bokeh/server/tornado.py:450:41: error: Argument 1 to "append" of "list" has incompatible type "tuple[dict[str, bool | dict[str, ApplicationContext] | str | None] | str | type[RequestHandler] | dict[str, Any], ...]"; expected "tuple[str, type[RequestHandler]] | tuple[str, type[RequestHandler], dict[str, Any]]" [arg-type]+ src/bokeh/server/tornado.py:453:37: error: Argument 1 to "append" of "list" has incompatible type "tuple[dict[str, bool | dict[str, ApplicationContext] | str | None] | str | type[RequestHandler] | dict[str, Any], ...]"; expected "tuple[str, type[RequestHandler]] | tuple[str, type[RequestHandler], dict[str, Any]]" [arg-type]+ src/bokeh/server/tornado.py: note: At top level: |
randolf-scholz commented Aug 16, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
The fromtypingimportIOdefadd_file(file: str|IO[bytes], filename: str|None=None) ->None: ... deftest(values: tuple[IO[bytes]] |tuple[IO[bytes], str]) ->None: add_file(*values) # master: OK ✅, PR: ❌Because it upcasts the type to So we need to deal with the case when each union member is a finite tuple differently
I think I can do option 2 for now and combine it with the equal size tuple case. |
hauntsaninja left a comment
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.
thanks for picking this up!
| return AnyType(TypeOfAny.from_error) | ||
| return self._make_iterable_instance_type(sol) | ||
| def as_iterable_type(self, typ: Type) -> IterableType | AnyType: |
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.
can we reuse logic from analyze_iterable_item_type_without_expression or similar?
| ret_type=target, | ||
| fallback=self.context.function_type, | ||
| ) | ||
| constraints = infer_constraints_for_callable( |
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.
Can mypy.infer.infer_function_type_arguments replace this and following line at once?
| actual_type = TupleType( | ||
| tuple_args, | ||
| # use Iterable[A | B | C] as the fallback type | ||
| fallback=Instance( |
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.
AFAIC we only use fallback to store builtins.tuple or a NamedTuple class, never anything else, and there might be an implicit assumption elsewhere that fallback is assignable to tuple[Any, ...], for example. Will anything break if you use builtins.tuple Instance here instead?
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.
tuple would be more appropriate; the issue is that currently ArgumentInferContext only knows about Mapping, Iterable and builtins.function.
Adding builtins.tuple there is possible, but will have to refactor all the fixtures to include a compatible tuple implementation.
| return self.as_iterable_type(p_t.upper_bound) | ||
| elif self.is_iterable(p_t): | ||
| # TODO: add a 'fast path' (needs measurement) that uses the map_instance_to_supertype | ||
| # mechanism? (Only if it works: gh-19662) |
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.
| iterator = iter(types) | ||
| typ = next(iterator) | ||
| if not isinstance(typ, TupleType): |
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.
If you refactor L449-L458 into a helper function, this method would look like
... size = _precise_tuple_len(typ) if size is None: return False return all(_precise_tuple_len(t) == size for t in iterator) | if isinstance(tt, TupleType): | ||
| if find_unpack_in_list(tt.items) is not None: | ||
| original_arg_type = self.accept(item.expr, ctx) | ||
| # convert arg type to one of TupleType, IterableType, AnyType or |
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.
...or? :)
sterliakov commented Aug 19, 2025
sterliakov commented Aug 19, 2025
This sounds good to me if it can be trivially implemented. If not, let's leave that part as-is or use the any fallback (your option 1) and defer to a separate PR, this PR is already quite long. |
randolf-scholz commented Sep 19, 2025
@sterliakov@hauntsaninja So an update on this. I turned out this whole thing was way more difficult to fix holistically than anticipated. The main issue is that when we want to deal with unions of tuples in a sensible manner, we already need to know the expected length of the argument when computing However, inside the I managed to work around this, building machinery that computes an abstract representation without actually materializing the Tuple type associated with a star argument. The resulting PR currently sits in my fork: randolf-scholz#6 I put quite a lot of effort into this, and I think the written code has generally high quality. But it has bloated in size to a monster PR (+3,838 −908) since I had to rewrite quite a few methods from scratch. What do you advise? |
ilevkivskyi commented Dec 14, 2025
@randolf-scholz could you please clarify what do you mean by |
randolf-scholz commented Dec 15, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@ilevkivskyi I'd need something like There are a few places where
At least in Footnotes
|
ilevkivskyi commented Jan 2, 2026
Yes, we (most likely me) are going to change this at some point. But it may be not for another couple months. So you can either wait until then, or "pipe through" required TypeInfos via callers to each of those call sites. |
| """ | ||
| from mypy.constraints import infer_constraints_for_callable | ||
| from mypy.nodes import ARG_POS | ||
| from mypy.solve import solve_constraints |
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.
TBH I don't like the nested imports. I know it may be hard, but I would recommend to try finding a way without introducing new import cycles.
randolf-scholz commented Jan 2, 2026
Well, #20472 is up now which solves these issues as I said by computing an abstract "Tuple Normal Form", that allows computing prefix and suffix length without access to This PR is probably dead unless these essential types become available globally. |
ilevkivskyi commented Jan 2, 2026
Well, yeah, that is huge. I guess the best way forward may be to simply wait until TypeInfos become globally available. |
Makes
*argsinference smarter by special casingUnionType*(tuple[A, B, C] | tuple[None|None|None])gets treated liketuple[A | None, B | None, C | None]insideArgTypeExpander(applies if all union member are fixed size tuples of equal length*(Iterable[X] | Iterable[Y])gets treated like*Iterable[X | Y]insideArgTypeExpander(applies if ① is not triggered)This gives some better inference in some cases:
See added unit tests for more examples.
mypyfails to correctly understand some iterable type in*args#19662tuple[int, *Ts, int]as anIterable[T]. #19659*argswhereargs: Union[Tuple[...], Tuple[...], ...]are _never_ checked [False Negative] #17161__iter__method yields false positive when used with star args (*) #14470See Also: #19650, cc @hauntsaninja
Dev notes:
If we detect that the union has non-tuple elements or tuples of different sizes, we upcast-reinterpret every union member as some
Iterable[T], then apply each union member, and then return the union of the results.Handling unions of finite size tuples with different sizes in infeasible, due to combinatoric explosion1
map_actuals_to_formalsadded special case for union of same sized tuples.ExpressionChecker.check_arg: added some extra logic that applies if thecallee_typeisUnpackType, but theactual_typegot expanded fromUnpack_typeto something else. Previously, this only worked "by accident" because the expander was guaranteed to returnAnyType(TypeOfAny.from_errors)in this case.ArgTypeExpander:NewTypealiasIterableTypeforIteralble[T], and helper functionsis_iterable,is_iterable_instance_type,_make_iterable_instance_type._solve_as_iterablefunction that uses the solver to interpret a type asIterable[T]as_iterable_typethat applies a few special cases (UnionType, etc. before calling the solver)parse_star_args_typethat converts the argument of*argsto one ofTupleType | IterableType | ParamSpecType | AnyTypevisit_tuple_expr: AppliesArgTypeExpander(*parse_star_args_type)to ensure consistent behavior across[*args],{*args}and(*args).Footnotes
consider
f(*x1, *x2, ..., *xn). If each $x_k$ is comprised of a union of $m_k$ differently sized tuples, then there are $m_1⋅m_2⋅…⋅m_k$ possible paths. ↩