Revert #88: FuncFactory NotSet signature defaults break signature consumers

i2/deco.py

@@ -91,17 +91,11 @@ class FuncFactory:
 
     >>> factory = FuncFactory(foo)
     >>> factory
-    <FuncFactory(foo)>(a=NotSet, b=NotSet, *, c=2) -> ...Callable[..., float]
+    <FuncFactory(foo)>(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
-    <FuncFactory(foo)>(b=NotSet, *, c=2) -> ...Callable[..., float]
+    <FuncFactory(foo)>(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
         )

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", "-"]

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)