From 31c1ac7486c7bf08559a1af83edd5724762fa5f3 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:46:52 +0100 Subject: [PATCH] Don't let a builtin name collision override a callable's real signature `_robust_signature_of_callable` consulted the curated `sigs_for_sigless_builtin_name` / `sigs_for_type_name` tables *before* trying `inspect.signature`. Those tables are keyed by `__name__` (and by type name), which is only a sound key for the C-level builtins they were written for. Any callable that merely shared a builtin's name was therefore handed the builtin's signature instead of its own: f = mk_place_holder_func(['chunker', 'wfs'], name='map') inspect.signature(f) # (chunker, wfs) <- correct Sig(f) # (func, iterable, /, *iterables) <- wrong Downstream this grew phantom parameters: every meshed DAG node built from a function named `map` sprouted an extra `iterables` input. The name-before-signature order was introduced to fix `operator` instances (itemgetter/attrgetter/methodcaller), which in Python 3.12+ do have a signature but a useless generic `(*args, **kwargs)`. That part is legitimate, so rather than demote the tables to a pure fallback (which would change resolution for `print`, `partialmethod`, the operator classes and the dunder wrappers, whose `signature` succeeds but whose curated entries are intentionally richer), the tables are now skipped only for callables that declare a signature of their own -- Python-defined functions/methods, and anything carrying an explicit `__signature__`. Genuine builtins declare neither, so they are unaffected. Verified behaviour-neutral: resolution is byte-identical before and after across all ~170 callables in `builtins`, `functools` and `operator` plus operator instances. i2's own suite passes (699), and the 47-package dependents sweep has an identical pass/fail set before and after (32 pass, 14 fail, all pre-existing). Claude-Session: https://claude.ai/code/session_01Kug7UUbVeCQgruvNXUq63c --- i2/signatures.py | 75 ++++++++++++++++++++++++++++++++----- i2/tests/test_signatures.py | 46 +++++++++++++++++++++++ 2 files changed, 111 insertions(+), 10 deletions(-) diff --git a/i2/signatures.py b/i2/signatures.py index 0df0a869..6e566578 100644 --- a/i2/signatures.py +++ b/i2/signatures.py @@ -97,7 +97,7 @@ ) from collections.abc import Callable, Iterable, Iterator, Mapping as MappingType from typing import KT, VT, T -from types import FunctionType +from types import FunctionType, MethodType from collections import defaultdict from operator import eq, attrgetter @@ -4311,6 +4311,47 @@ def decorator(targ_func): # ############################################################################ +#: Callable kinds that are defined in Python (as opposed to C-level builtins) and +#: therefore always carry authoritative signature information of their own. +PYTHON_DEFINED_CALLABLE_TYPES = (FunctionType, MethodType) + + +def _declares_own_signature(callable_obj: Callable) -> bool: + """Whether ``callable_obj`` carries authoritative signature information of its own. + + The ``sigs_for_sigless_builtin_name`` and ``sigs_for_type_name`` tables are keyed by + name, which is only a sound key for the C-level builtins they were written for. An + object that declares its own signature must never be overridden by a name collision. + + A Python-defined function knows its own signature: + + >>> def map(chunker, wfs): # shadows the ``map`` builtin + ... ... + >>> _declares_own_signature(map) + True + + So does any object carrying an explicit ``__signature__`` (which is how i2 itself + stamps signatures onto ``functools.partial`` objects and other wrappers): + + >>> from functools import partial + >>> from inspect import signature + >>> p = partial(lambda a, b: None, 1) + >>> _declares_own_signature(p) + False + >>> p.__signature__ = signature(lambda chunker, wfs: None) + >>> _declares_own_signature(p) + True + + Genuine builtins declare nothing, so the curated tables still apply to them: + + >>> _declares_own_signature(print) + False + """ + return getattr(callable_obj, "__signature__", None) is not None or isinstance( + callable_obj, PYTHON_DEFINED_CALLABLE_TYPES + ) + + # TODO: Might want to monkey-patch inspect._signature_from_callable to use # sigs_for_sigless_builtin_name def _robust_signature_of_callable(callable_obj: Callable) -> Signature: @@ -4330,16 +4371,30 @@ def _robust_signature_of_callable(callable_obj: Callable) -> Signature: ... ) # doesn't have one, so will return a blanket one + A callable that carries its own signature information is never overridden by the + curated tables, even if its ``__name__`` happens to collide with a builtin's: + + >>> def map(chunker, wfs): # a Python function that shadows the ``map`` builtin + ... ... + >>> _robust_signature_of_callable(map) + + """ - # First check if we have a custom signature for this type/object - # This is important for operator instances that might have generic signatures in Python 3.12+ - obj_name = getattr(callable_obj, "__name__", None) - if obj_name in sigs_for_sigless_builtin_name: - return sigs_for_sigless_builtin_name[obj_name] or DFLT_SIGNATURE - - type_name = getattr(type(callable_obj), "__name__", None) - if type_name in sigs_for_type_name: - return sigs_for_type_name[type_name] or DFLT_SIGNATURE + # The curated tables are keyed by *name*, which is only a sound key for the + # C-level builtins they were written for. Consulting them for a callable that + # knows its own signature would let a mere name collision (e.g. a Python function + # named ``map``) replace a correct signature with the builtin's one. + if not _declares_own_signature(callable_obj): + # Check for a curated signature for this object/type. This must precede + # ``signature`` because operator instances (itemgetter, attrgetter, + # methodcaller) do have a signature in Python 3.12+, but a useless generic one. + obj_name = getattr(callable_obj, "__name__", None) + if obj_name in sigs_for_sigless_builtin_name: + return sigs_for_sigless_builtin_name[obj_name] or DFLT_SIGNATURE + + type_name = getattr(type(callable_obj), "__name__", None) + if type_name in sigs_for_type_name: + return sigs_for_type_name[type_name] or DFLT_SIGNATURE # Try to get the signature normally try: diff --git a/i2/tests/test_signatures.py b/i2/tests/test_signatures.py index 811d7ff0..7162a471 100644 --- a/i2/tests/test_signatures.py +++ b/i2/tests/test_signatures.py @@ -2400,3 +2400,49 @@ def _test_call(call, expected_output): call() else: assert call() == expected_output + + +# --------------------------------------------------------------------------------- +# Regression: a name collision with a builtin must not override a real signature. +# `sigs_for_sigless_builtin_name` is keyed by __name__ alone, so consulting it before +# `inspect.signature` gave any callable named e.g. `map` the *builtin* map's signature, +# growing phantom parameters (it made meshed DAG nodes sprout an `iterables` input). + + +def test_builtin_name_collision_does_not_override_own_signature(): + """A callable named after a builtin keeps its own signature.""" + + # A plain Python function whose name shadows a builtin + def map(chunker, wfs): # noqa: A001 - shadowing is the point of the test + return chunker, wfs + + assert str(Sig(map)) == "(chunker, wfs)" + assert str(_robust_signature_of_callable(map)) == "(chunker, wfs)" + + # An object carrying an explicit __signature__ (how i2 stamps partials/wrappers) + placeholder = partial(lambda *a, **kw: None) + placeholder.__signature__ = signature(lambda chunker, wfs: None) + placeholder.__name__ = "map" + + assert str(Sig(placeholder)) == "(chunker, wfs)" + assert str(_robust_signature_of_callable(placeholder)) == "(chunker, wfs)" + + +def test_sigless_builtins_still_get_their_curated_signatures(): + """The curated table must still serve the genuine builtins it was written for.""" + # `map` itself has no introspectable signature, so the curated one must be used + with pytest.raises(ValueError): + signature(map) + assert str(Sig(map)) == str(sigs_for_sigless_builtin_name["map"]) + + # `print` has a curated signature that intentionally differs from the introspected + # one, and must keep winning + assert str(_robust_signature_of_callable(print)) == str( + sigs_for_sigless_builtin_name["print"] + ) + + # operator instances have a useless generic signature in 3.12+, so the curated + # per-type signature must keep taking precedence over `inspect.signature` + from operator import itemgetter + + assert str(_robust_signature_of_callable(itemgetter(1))) != "(*args, **kwargs)"