| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hm there is still some issue to be solved with Unpack. The flatten_nested_tuples method helps./ I think one could determine the correct upcast by using the solver, but I discovered some inconsistencies: #19652 |
Sorry, something went wrong.
|
This really does catch quite a few false negatives, for example: Repro of homeassistant/components/govee_light_local https://mypy-play.net/?mypy=latest&python=3.12&gist=3bf7d56a2bfbc1fc23be62ef521dc613 def set_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_state
if color:
set_rgb_color(*color) # MASTER: no error ❌, PR: raises [arg-type] ✅Repro of xarray/tests/test_namedarray: https://mypy-play.net/?mypy=latest&python=3.12&gist=9999ca2cb89e849978c979c1e4cc9bca from typing import Iterable, Hashable
type _Dim = Hashable
type _Dims = tuple[_Dim, ...]
type _DimsLike = str | Iterable[_Dim]
def permute_dims(*dim: Iterable[_Dim]) -> None: ...
def test(dims: _DimsLike) -> None:
permute_dims(*dims) # master: no error ❌, PR: raises [arg-type] ✅Repro of src/bokeh/server/tornado.py:450: https://mypy-play.net/?mypy=latest&python=3.12&gist=0df919e6986b3fcbdb9d7c8727fb8d48 from typing import Any
class RequestHandler: ...
type RouteContext = dict[str, Any]
type URLRoutes = list[
tuple[str, type[RequestHandler]] |
tuple[str, type[RequestHandler], RouteContext],
]
def demo(
all_patterns: URLRoutes,
extra_patterns: URLRoutes,
toplevel_patterns: URLRoutes,
prefix: str,
data: dict[str, Any],
) -> None:
for p in extra_patterns + toplevel_patterns:
prefixed_pat = (prefix + p[0], *p[1:], data)
all_patterns.append(prefixed_pat) # MASTER: false negative, PR: [arg-type] |
Sorry, something went wrong.
|
I think this is ready for initial review. |
Sorry, something went wrong.
This reverts commit e8dcf88.
Sorry, something went wrong.
|
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:
|
Sorry, something went wrong.
|
The src/werkzeug/test.py is a case where the PR introduces a false positive: https://mypy-play.net/?mypy=latest&python=3.12&gist=eb790692cdcb803373269187d9ad0a4a from typing import IO
def add_file(file: str | IO[bytes], filename: str | None = None) -> None: ...
def test(values: tuple[IO[bytes]] | tuple[IO[bytes], str]) -> None:
add_file(*values) # master: OK ✅, PR: ❌Because it upcasts the type to typing.Iterable[typing.IO[builtins.bytes] | builtins.str], whereas master treats it like tuple[Any, ...]. 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. |
Sorry, something went wrong.
There was a problem hiding this comment.
thanks for picking this up!
Sorry, something went wrong.
| 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.
can we reuse logic from analyze_iterable_item_type_without_expression or similar?
Sorry, something went wrong.
| ret_type=target, | ||
| fallback=self.context.function_type, | ||
| ) | ||
| constraints = infer_constraints_for_callable( |
There was a problem hiding this comment.
Can mypy.infer.infer_function_type_arguments replace this and following line at once?
Sorry, something went wrong.
| actual_type = TupleType( | ||
| tuple_args, | ||
| # use Iterable[A | B | C] as the fallback type | ||
| fallback=Instance( |
There was a problem hiding this comment.
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?
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
| 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.
Sorry, something went wrong.
|
|
||
| iterator = iter(types) | ||
| typ = next(iterator) | ||
| if not isinstance(typ, TupleType): |
There was a problem hiding this comment.
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)
Sorry, something went wrong.
| 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.
...or? :)
Sorry, something went wrong.
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
@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 formal_to_actual. However, inside the argmap.map_actuals_to_formals, we do not necessarily have access to the TypeInfo of typing.Iterable and builtins.tuple, which this PR uses to determine the actual length. 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? |
Sorry, something went wrong.
|
@randolf-scholz could you please clarify what do you mean by TypeInfos being not available in map_actuals_to_formals()? I would expect that all callers (essentially expression checker + few others) should have access to those TypeInfos, so you should be able to pass them as an additional argument there. Is it not the case for some reason? |
Sorry, something went wrong.
|
@ilevkivskyi I'd need something like infer.ArgumentInferContext inside map_actuals_to_formals that has access to the TypeInfo of: typing.Iterable, builtins.tuple and builtins.function. There are a few places where map_actuals_to_formals is used, and it is unclear to me how I'd get access to these TypeInfo:
At least in functools.py I could use ctx.api.named_generic_type to get them. To be honest, it feels a bit weird that certain types1 that are used by core language syntax are not always available, e.g. with a global variable or something, but need to be passed around with helpers like ArgumentInferContext. Footnotes
|
Sorry, something went wrong.
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. |
Sorry, something went wrong.
| """ | ||
| 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.
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.
Sorry, something went wrong.
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 infer.ArgumentInferContext. But that PR has become extremely large, so reviewing it will be challenging, and I am not sure how to cut it up into smaller bits. This PR is probably dead unless these essential types become available globally. |
Sorry, something went wrong.
|
Well, yeah, that is huge. I guess the best way forward may be to simply wait until TypeInfos become globally available. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Makes *args inference smarter by special casing UnionType
This gives some better inference in some cases:
See added unit tests for more examples.
See 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_formals added special case for union of same sized tuples.
ExpressionChecker.check_arg: added some extra logic that applies if the callee_type is UnpackType, but the actual_type got expanded from Unpack_type to something else. Previously, this only worked "by accident" because the expander was guaranteed to return AnyType(TypeOfAny.from_errors) in this case.
ArgTypeExpander:
visit_tuple_expr: Applies ArgTypeExpander(*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. ↩