Skip to content

Fix false positive on callables with non-constant parameter mappings - #4570

Open
alexander-beedie wants to merge 1 commit into
meta:mainfrom
alexander-beedie:non-constant-param-mapping
Open

alexander-beedie wants to merge 1 commit into
meta:mainfrom
alexander-beedie:non-constant-param-mapping

Conversation

@alexander-beedie

@alexander-beedie alexander-beedie commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #3587.

Overview

A caller of a function does not have to pass its positional-or-keyword parameters positionally - it can pass any prefix positionally, and the rest by name.

When checking whether got is assignable to want, only the one mapping where every such parameter of want is passed positionally was checked (which also requires the two signatures to agree on parameter names), so callables that satisfy want under a different mapping were rejected.

Solution

  • Treat the parameter list check as a question about all of want's calling conventions rather than just one of them.

  • If the as-written mapping fails, is_subset_param_list retries the check once per split, moving the positional/by-name boundary one parameter to the right each time, and accepts only if every split is assignable.

  • Because Python forbids positional arguments after keyword arguments, the splits are a prefix chain, so there are only n+1 of them (rather than 2^n).

  • The retry only runs when the as-written mapping fails and the comparison is not gradual, so accepting comparisons cost what they did before, and the splits speculatively pin type variables, so they run inside Solver::with_snapshot and roll back on failure 👍

Test Plan

  • Existing unit tests all pass.
  • Added 6-7 more tests (including the example given in the original Issue).

@meta-codesync

meta-codesync Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D116281213. (Because this pull request was imported automatically, there will not be any future comments.)

@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from d29f610 to 2b0c64f Compare August 25, 2026 08:20
@github-actions github-actions Bot added size/l and removed size/l labels Aug 25, 2026
@github-actions

This comment has been minimized.

@alexander-beedie

Copy link
Copy Markdown
Contributor Author

Rebased/resolved 👍

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from 2b0c64f to bdd7008 Compare September 8, 2026 06:28
@github-actions github-actions Bot added size/l and removed size/l labels Sep 8, 2026
@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from bdd7008 to 1fced0a Compare September 10, 2026 06:37
@github-actions github-actions Bot added size/l and removed size/l labels Sep 10, 2026
@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from 1fced0a to 5d1d3eb Compare September 18, 2026 13:12
@github-actions github-actions Bot added size/l and removed size/l labels Sep 18, 2026
@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from 5d1d3eb to 105230c Compare September 21, 2026 09:35
@github-actions github-actions Bot added size/l and removed size/l labels Sep 21, 2026
@alexander-beedie

alexander-beedie commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

@samwgoldman - rebased/resolved again :)
(and added an extra test, covering a duplicate-argument acceptance case) 👌

@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from 105230c to d140073 Compare September 30, 2026 15:41
@github-actions github-actions Bot removed the size/l label Sep 30, 2026
@codspeed

codspeed Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 35 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing alexander-beedie:non-constant-param-mapping (6bb93c6) with main (0e44796)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from d140073 to 71a1110 Compare October 2, 2026 06:19
@github-actions github-actions Bot added size/l and removed size/l labels Oct 2, 2026
@github-actions

This comment has been minimized.

@alexander-beedie
alexander-beedie force-pushed the non-constant-param-mapping branch from 71a1110 to 6bb93c6 Compare October 7, 2026 08:32
@github-actions github-actions Bot added size/l and removed size/l labels Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Diff from mypy_primer, showing the effect of this PR on open source code:

============================================================
SUMMARY
============================================================
Total: +2 new errors, -43 fixed errors
By preset: +2/-43 (default), +2/-43 (strict)

Projects with changes (9):
  openlibrary: +0 -1
  spark: +0 -14
  scipy: +1 -0
  django-stubs: +0 -16
  hydra-zen: +0 -2
  ibis: +0 -1
  static-frame: +0 -4
  werkzeug: +0 -4
  xarray: +1 -1
============================================================

FULL DIFF DETAILS
------------------------------------------------------------

openlibrary (https://github.com/internetarchive/openlibrary)
- ERROR openlibrary/core/models.py:1052:9-28: Class member `User._get_lists_uncached` overrides parent class `Thing` in an inconsistent manner [bad-override]

spark (https://github.com/apache/spark)
- ERROR python/pyspark/sql/classic/dataframe.py:2078:9-16: Implementation signature `(self: Self@DataFrameNaFunctions, to_replace: dict[Unknown, Unknown] | list[Unknown], value: list[Unknown] | Unknown | None = ..., subset: list[str] | None = None) -> Unknown` does not accept all arguments that overload signature `(self: Self@DataFrameNaFunctions, to_replace: dict[Unknown, Unknown], subset: list[str] | None = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/context.py:369:9-24: Implementation signature `(self: Self@SQLContext, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SQLContext, data: Iterable[Unknown] | Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/context.py:380:9-24: Implementation signature `(self: Self@SQLContext, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SQLContext, data: Unknown, samplingRatio: float | None = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/context.py:385:9-24: Implementation signature `(self: Self@SQLContext, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SQLContext, data: Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/pandas/conversion.py:570:9-24: Implementation signature `(self: Self@SparkConversionMixin, data: Unknown, schema: list[str] | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkConversionMixin, data: Unknown, samplingRatio: float | None = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/pandas/conversion.py:575:9-24: Implementation signature `(self: Self@SparkConversionMixin, data: Unknown, schema: list[str] | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkConversionMixin, data: Unknown, samplingRatio: float | None = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/pandas/conversion.py:580:9-24: Implementation signature `(self: Self@SparkConversionMixin, data: Unknown, schema: list[str] | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkConversionMixin, data: Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/pandas/conversion.py:588:9-24: Implementation signature `(self: Self@SparkConversionMixin, data: Unknown, schema: list[str] | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkConversionMixin, data: Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/session.py:1386:9-24: Implementation signature `(self: Self@SparkSession, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkSession, data: Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/session.py:1394:9-24: Implementation signature `(self: Self@SparkSession, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkSession, data: Iterable[Unknown], schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/session.py:1402:9-24: Implementation signature `(self: Self@SparkSession, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkSession, data: Unknown, samplingRatio: float | None = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/session.py:1407:9-24: Implementation signature `(self: Self@SparkSession, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkSession, data: Unknown, samplingRatio: float | None = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/session.py:1412:9-24: Implementation signature `(self: Self@SparkSession, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkSession, data: Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]
- ERROR python/pyspark/sql/session.py:1420:9-24: Implementation signature `(self: Self@SparkSession, data: Iterable[Any] | Unknown, schema: str | Unknown | None = None, samplingRatio: float | None = None, verifySchema: bool = True) -> Unknown` does not accept all arguments that overload signature `(self: Self@SparkSession, data: Unknown, schema: str | Unknown, verifySchema: bool = ...) -> Unknown` accepts [inconsistent-overload]

scipy (https://github.com/scipy/scipy)
+ ERROR scipy/integrate/_quad_vec.py:25:9-15: Class member `LRUDict.update` overrides parent class `OrderedDict` in an inconsistent manner [bad-override]

django-stubs (https://github.com/typeddjango/django-stubs)
- ERROR django-stubs/test/client.pyi:291:9-12: Class member `Client.get` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:303:9-13: Class member `Client.post` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:316:9-13: Class member `Client.head` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:328:9-16: Class member `Client.options` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:341:9-12: Class member `Client.put` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:354:9-14: Class member `Client.patch` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:367:9-15: Class member `Client.delete` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:380:9-14: Class member `Client.trace` overrides parent class `_RequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:410:15-18: Class member `AsyncClient.get` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:422:15-19: Class member `AsyncClient.post` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:435:15-19: Class member `AsyncClient.head` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:447:15-22: Class member `AsyncClient.options` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:460:15-18: Class member `AsyncClient.put` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:473:15-20: Class member `AsyncClient.patch` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:486:15-21: Class member `AsyncClient.delete` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]
- ERROR django-stubs/test/client.pyi:499:15-20: Class member `AsyncClient.trace` overrides parent class `_AsyncRequestFactory` in an inconsistent manner [bad-override]

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- ERROR src/hydra_zen/wrapper/_implementations.py:1472:9-17: Implementation signature `(self: Self@ZenStore, __target: F | None = None, /, **kw: Any) -> F | Self@ZenStore` does not accept all arguments that overload signature `(self: Self@ZenStore, __target: F, *, name: ((F) -> str) | str = ..., group: ((F) -> GroupName) | str | None = ..., package: ((F) -> str) | str | None = ..., provider: str | None = ..., to_config: (F) -> Any = ..., **to_config_kw: Any) -> F` accepts [inconsistent-overload]
- ERROR src/hydra_zen/wrapper/_implementations.py:1485:9-17: Implementation signature `(self: Self@ZenStore, __target: F | None = None, /, **kw: Any) -> F | Self@ZenStore` does not accept all arguments that overload signature `(self: Self@ZenStore, __target: None = None, *, name: ((Any) -> str) | str = ..., group: ((Any) -> GroupName) | str | None = ..., package: ((Any) -> str) | str | None = ..., provider: str | None = ..., to_config: (Any) -> Any = ..., **to_config_kw: Any) -> Self@ZenStore` accepts [inconsistent-overload]

ibis (https://github.com/ibis-project/ibis)
- ERROR ibis/expr/api.py:856:5-14: Implementation signature `(value_or_year: Unknown, month: Unknown | None = None, day: Unknown | None = None, hour: Unknown | None = None, minute: Unknown | None = None, second: Unknown | None = None, /, timezone: Unknown | None = None) -> Scalar | Unknown` does not accept all arguments that overload signature `(value_or_year: Any, /, timezone: str | None = None) -> TimestampValue` accepts [inconsistent-overload]

static-frame (https://github.com/static-frame/static-frame)
- ERROR static_frame/core/frame.py:5343:9-17: Implementation signature `(self: Self@Frame, row_key: TILocSelector = None, column_key: TILocSelector = None) -> Any` does not accept all arguments that overload signature `(self: Self@Frame, column_key: TILocSelectorOne) -> Series` accepts [inconsistent-overload]
- ERROR static_frame/core/frame.py:5346:9-17: Implementation signature `(self: Self@Frame, row_key: TILocSelector = None, column_key: TILocSelector = None) -> Any` does not accept all arguments that overload signature `(self: Self@Frame, column_key: TILocSelectorMany) -> Self@Frame` accepts [inconsistent-overload]
- ERROR static_frame/core/type_blocks.py:3216:9-17: Implementation signature `(self: Self@TypeBlocks, row_key: TILocSelector = None, column_key: TILocSelector = None) -> Any` does not accept all arguments that overload signature `(self: Self@TypeBlocks, column_key: TILocSelectorOne) -> TypeBlocks` accepts [inconsistent-overload]
- ERROR static_frame/core/type_blocks.py:3219:9-17: Implementation signature `(self: Self@TypeBlocks, row_key: TILocSelector = None, column_key: TILocSelector = None) -> Any` does not accept all arguments that overload signature `(self: Self@TypeBlocks, column_key: TILocSelectorMany) -> TypeBlocks` accepts [inconsistent-overload]

werkzeug (https://github.com/pallets/werkzeug)
- ERROR src/werkzeug/datastructures/headers.py:122:9-12: Implementation signature `(self: Self@Headers, key: str, default: str | T | None = None, type: ((str) -> T) | None = None) -> str | T | None` does not accept all arguments that overload signature `(self: Self@Headers, key: str, type: (str) -> T) -> T | None` accepts [inconsistent-overload]
- ERROR src/werkzeug/datastructures/structures.py:75:9-12: Implementation signature `(self: Self@_TypeConversionDict, key: K, default: V | T | None = None, type: ((V) -> T) | None = None) -> V | T | None` does not accept all arguments that overload signature `(self: Self@_TypeConversionDict, key: str, type: (V) -> T) -> T | None` accepts [inconsistent-overload]
- ERROR src/werkzeug/datastructures/structures.py:270:9-12: Implementation signature `(self: Self@MultiDict, key: K, default: V | T | None = None, type: ((V) -> T) | None = None) -> V | T | None` does not accept all arguments that overload signature `(self: Self@MultiDict, key: str, type: (V) -> T) -> T | None` accepts [inconsistent-overload]
- ERROR src/werkzeug/datastructures/structures.py:640:9-12: Implementation signature `(self: Self@CombinedMultiDict, key: K, default: V | T | None = None, type: ((V) -> T) | None = None) -> V | T | None` does not accept all arguments that overload signature `(self: Self@CombinedMultiDict, key: str, type: (V) -> T) -> T | None` accepts [inconsistent-overload]

xarray (https://github.com/pydata/xarray)
- ERROR xarray/core/resample.py:373:9-14: Class member `DataArrayResample.apply` overrides parent class `DataArrayGroupByBase` in an inconsistent manner [bad-override-param-name]
+ ERROR xarray/core/resample.py:373:9-14: Class member `DataArrayResample.apply` overrides parent class `DataArrayGroupByBase` in an inconsistent manner [bad-override]

@stroxler stroxler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This now needs a rebase. In addition, code review flagged a few potential issues we should look at before merge:

  1. **kwargs can duplicate a positionally filled parameter. The PR accepts Want(a: int, **kwargs: int) backed by got(b: int = 0, **kwargs: int). Calling w(1, b=2) is legal through Want but raises TypeError in got. The collision check needs to consider names supplied through arbitrary kwargs.

  2. Successful splits can produce an invalid generic specialization. A callback accepting str positionally and int by keyword gets inferred as Want[int | str], incorrectly allowing either type in either position. The final specialization must satisfy every split—not merely accumulate individually successful constraints.

  3. Failed fallback checks leak inference state. A rejected callback pins an unrelated empty list’s element type to str, causing a subsequent xs.append(1) error. Snapshotting needs to include variables reachable through existing bounds and answers.

For cleanup, the AI reviewer also suggests recording positional bindings during the walk instead of reconstructing them afterward with zip.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False-positive [inconsistent-overload] with non-constant parameter mapping

4 participants