diff --git a/i2/deco.py b/i2/deco.py index 437c45cc..4964828c 100644 --- a/i2/deco.py +++ b/i2/deco.py @@ -91,17 +91,11 @@ class FuncFactory: >>> factory = FuncFactory(foo) >>> factory - (a=NotSet, b=NotSet, *, c=2) -> ...Callable[..., float] + (a, b, *, 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``: @@ -120,7 +114,7 @@ class FuncFactory: >>> factory_no_a = FuncFactory(foo, exclude=['a']) >>> factory_no_a - (b=NotSet, *, c=2) -> ...Callable[..., float] + (b, *, c=2) -> ...Callable[..., float] >>> g = factory_no_a(2, 3) # equivalent to ``factory(b=2, c=3)`` as no ``a`` here >>> g(10) 23 @@ -175,17 +169,15 @@ def __init__(self, func, *, include=(), exclude=()): self.func_sig = func_sig self.factory_sig = actual_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} - ) + 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 @classmethod def wrap(cls, include=(), exclude=()): @@ -196,9 +188,8 @@ def _process_args_and_kwargs(self, args, kwargs): _kwargs = self.factory_sig.map_arguments( args, kwargs, allow_partial=True, ignore_kind=True ) - # 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} + # 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} __args, __kwargs = self.func_sig.mk_args_and_kwargs( _kwargs, allow_partial=True, ignore_kind=False ) diff --git a/i2/tests/test_deco.py b/i2/tests/test_deco.py new file mode 100644 index 00000000..b7523787 --- /dev/null +++ b/i2/tests/test_deco.py @@ -0,0 +1,39 @@ +"""Tests for ``i2.deco``. + +In particular, pins the *signature contract* of ``FuncFactory`` instances, which +signature-driven consumers (UI generators like ``front``/``streamlitfront``, HTTP/OpenAPI +generators like ``py2http``) introspect. +""" + +import inspect + +from i2 import FuncFactory, Sig + + +def _g(wf, chk_size: int, name: str, *, sep="-") -> list: + return [wf, chk_size, name, sep] + + +def test_func_factory_signature_keeps_required_params_required(): + """Regression guard for the revert of i2mint/i2#88 (see i2mint/i2#48). + + Giving the factory's non-defaulted params a ``NotSet`` sentinel default made + signature consumers treat the sentinel as a real default value: ``front`` input + elements crashed (``int(NotSet)``) or prefilled ``"NotSet"``, and ``py2http`` + produced OpenAPI specs that were not JSON-serializable. Until those consumers + understand the sentinel, a ``FuncFactory``'s signature must show exactly the + wrapped function's defaults (no sentinel ones). + """ + factory = FuncFactory(_g, exclude="wf") + sig = Sig(factory) + + assert list(sig.names) == ["chk_size", "name", "sep"] + assert list(sig.required_names) == ["chk_size", "name"] + assert sig.defaults == {"sep": "-"} + g_params = inspect.signature(_g).parameters + for name, p in inspect.signature(factory).parameters.items(): + assert p.default is g_params[name].default + + # The factory itself can still be called with none to all of the arguments + assert factory()(1, 2, "a") == [1, 2, "a", "-"] + assert factory(chk_size=3)(1, name="b") == [1, 3, "b", "-"] diff --git a/i2/tests/test_wrapper.py b/i2/tests/test_wrapper.py index d9facb26..60ccdca3 100644 --- a/i2/tests/test_wrapper.py +++ b/i2/tests/test_wrapper.py @@ -278,10 +278,7 @@ def test_rm_params(): allow_partial=True, # wouldn't work without this ) - # `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)" + assert str(Sig(mk_chunker)) == "(chk_size: int)" wf = range(7) chunker = mk_chunker(chk_size=3)