Revert #88: FuncFactory NotSet signature defaults break signature consumers - #89
Merged
Merged
Conversation
…ired params (#88)" This reverts commit 70125c0. Post-merge adversarial review found that putting a NotSet sentinel default on every FuncFactory param breaks signature-driven consumers in the fleet: - front input elements: an int/float-annotated param crashes at element init (int(NotSet) -> TypeError); str params prefill the literal text "NotSet", which is then passed as a real value. - py2http: the OpenAPI spec built for a FuncFactory is no longer JSON-serializable (Object of type Sentinel), and its params stop being listed as required. Adds a regression test pinning the FuncFactory signature contract (the wrapped function's defaults, no sentinel ones) until consumers are made sentinel-aware. Re-landing plan is in #48. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 tasks done
thorwhalen
added a commit
that referenced
this pull request
Sep 22, 2026
Module map centered on signatures.py/deco.py/wrapper.py, verified test command (legacy isee CI, no lint gate), the i2#88/#89 breaking-change postmortem as a standing invariant, and the 50-package dependent list. Docs-only. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
round-3 review: one defect, now fixed. This PR's body says "#88 (which closed #48)", and GitHub read "closed #48" as a closing keyword, so merging it re-closed #48 with nothing landed. #48 is reopened. Also checked: the code is byte-identical to pre-#88 (the only other diffs are setup.cfg and the new test), and test_deco.py fails on #88's code (required_names == []) and passes on master (762 passed). |
This was referenced Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Post-merge adversarial review of #88 (which closed #48). #88 gave every non-defaulted
FuncFactoryparam aNotSetsentinel default in the factory's__signature__. The call behaviour is unchanged, but the signature is public API, and several i2 dependents build things from it. They read the sentinel as a real default value:frontIntInputBase/FloatInputBasefor anint/floatparam of aFuncFactory0TypeError: int() argument must be ... not 'Sentinel'at element initfrontTextInputBasefor astrparam'''NotSet', which gets submitted as a real string (not the sentinel, soFuncFactorydoes not filter it)py2http.service.mk_routes_and_openapi_specs([factory])TypeError: Object of type Sentinel is not JSON serializable, and the params stop being listed as requiredstreamlitfront'sexamples/data_prep.py(andknow's equivalents) feedSig(FuncFactory(...))straight intoKwargsInput, so they hit thefrontpath.The test suites of
front,streamlitfront,meshed,juandslanggive the same result before and after #88. None of them covers aFuncFactorysignature going through a UI or schema. That is why #88's dependents check did not catch this.What
i2/deco.py,i2/tests/test_wrapper.py).i2/tests/test_deco.py. It pins theFuncFactorysignature contract: the factory shows exactly the wrapped function's defaults, with no sentinel defaults, and it can still be called with none to all of the args. The test fails on fix: make FuncFactory signature show NotSet defaults for required params #88's code and passes here.The plan for re-landing #48 without breaking consumers is written in #48.
Gate
pytest --doctest-modules i2: 762 passed, 2 xfailedblack --check -Son the changed files: cleanReview: independent post-merge review of #88. This revert itself is self-reviewed only (the reviewing session was told not to spawn sub-agents).
🤖 Generated with Claude Code