From 70b1cc2c70ed16274ba1caff34ae2412bc2f8eb9 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 22 Sep 2026 13:19:26 +0000 Subject: [PATCH] fix: make FuncFactory signature show NotSet defaults for required params FuncFactory can be called with anywhere from none to all of the underlying function's arguments, but its signature previously showed the required-looking params (no default) as if they were mandatory to call the factory itself -- which they aren't. This activates the NotSet-sentinel approach that was already sketched out (commented) in the code and prototyped on the unmerged branch origin/48-using-a-notset-sentinel-to-solve-make-funcfactory-instances-signature-more-truthful-about-argument-requirements, with one correction: avoid re-indexing the signature through `Sig.__getitem__` a second time, since that silently drops the return annotation (a separate, pre-existing bug in `Sig.__getitem__`/ `from_params`, not addressed here). Updates the two affected FuncFactory doctests and the test_rm_params regression test (i2mint/i2#44) to reflect the new, more truthful signature display. Closes #48 Co-Authored-By: Claude Sonnet 5 --- i2/deco.py | 35 ++++++++++++++++++++++------------- i2/tests/test_wrapper.py | 5 ++++- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/i2/deco.py b/i2/deco.py index 4964828c..437c45cc 100644 --- a/i2/deco.py +++ b/i2/deco.py @@ -91,11 +91,17 @@ class FuncFactory: >>> factory = FuncFactory(foo) >>> factory - (a, b, *, c=2) -> ...Callable[..., float] + (a=NotSet, b=NotSet, *, c=2) -> ...Callable[..., float] (Note that the repr even reuses ``foo``'s return annotation to tell us that our factory will return a callable that returns that type (if the annotation is a type). + Note also that ``a`` and ``b`` show a ``NotSet`` default, even though ``foo`` + itself requires them: that's ``FuncFactory`` being truthful about what calling + the *factory* requires (nothing -- it can be called with anywhere from none to + all of the underlying function's arguments), as opposed to what calling the + function it produces requires. + An instance of ``FuncFactory`` is a factory of functions, that is, it can make functions for you based on the instance's underlying ``func``: @@ -114,7 +120,7 @@ class FuncFactory: >>> factory_no_a = FuncFactory(foo, exclude=['a']) >>> factory_no_a - (b, *, c=2) -> ...Callable[..., float] + (b=NotSet, *, c=2) -> ...Callable[..., float] >>> g = factory_no_a(2, 3) # equivalent to ``factory(b=2, c=3)`` as no ``a`` here >>> g(10) 23 @@ -169,15 +175,17 @@ def __init__(self, func, *, include=(), exclude=()): self.func_sig = func_sig self.factory_sig = actual_factory_sig - self.__signature__ = actual_factory_sig # TODO: Delete when #48 solved - # TODO: Uncomment below to resolved https://github.com/i2mint/i2/issues/48) - ## Add NotSet default to all non-defaulted params: - ## (To see why, go to See https://github.com/i2mint/i2/issues/48) - # shown_factory_sig = actual_factory_sig.ch_defaults( - # **{name: NotSet for name in actual_factory_sig.required_names} - # ) - # shown_factory_sig = shown_factory_sig[self.include] - # self.__signature__ = shown_factory_sig + # Give the (required) params that don't have a default a `NotSet` default, + # so that the factory's signature is truthful about what's actually required + # to call `self.func` (as opposed to what's required to call the factory + # itself, which can be called with anything from none to all of these + # params -- see https://github.com/i2mint/i2/issues/48). + # Note: `actual_factory_sig` is already restricted to `self.include` (see + # above), so we don't re-index it here -- doing so would needlessly go + # through `Sig.__getitem__`, which drops the return annotation. + self.__signature__ = actual_factory_sig.ch_defaults( + **{name: NotSet for name in actual_factory_sig.required_names} + ) @classmethod def wrap(cls, include=(), exclude=()): @@ -188,8 +196,9 @@ def _process_args_and_kwargs(self, args, kwargs): _kwargs = self.factory_sig.map_arguments( args, kwargs, allow_partial=True, ignore_kind=True ) - # Uncomment below to resolved https://github.com/i2mint/i2/issues/48) - # _kwargs = {k: v for k, v in _kwargs.items() if v is not NotSet} + # Drop the `NotSet`-defaulted params that weren't actually given a value + # (see https://github.com/i2mint/i2/issues/48): + _kwargs = {k: v for k, v in _kwargs.items() if v is not NotSet} __args, __kwargs = self.func_sig.mk_args_and_kwargs( _kwargs, allow_partial=True, ignore_kind=False ) diff --git a/i2/tests/test_wrapper.py b/i2/tests/test_wrapper.py index 60ccdca3..d9facb26 100644 --- a/i2/tests/test_wrapper.py +++ b/i2/tests/test_wrapper.py @@ -278,7 +278,10 @@ def test_rm_params(): allow_partial=True, # wouldn't work without this ) - assert str(Sig(mk_chunker)) == "(chk_size: int)" + # `chk_size` shows a `NotSet` default: FuncFactory can be called with anywhere + # from none to all of its underlying function's arguments, so its signature + # shouldn't claim `chk_size` is strictly required (see i2mint/i2#48). + assert str(Sig(mk_chunker)) == "(chk_size: int = NotSet)" wf = range(7) chunker = mk_chunker(chk_size=3)