funcutils: forward defaulted args as keyword args in wraps invocation#412
Open
gaoflow wants to merge 1 commit into
Open
funcutils: forward defaulted args as keyword args in wraps invocation#412gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
…tion `FunctionBuilder.get_invocation_str()` was passing every regular argument positionally (`return _call(x, y, msg)`). When a caller supplied a defaulted parameter by keyword (`f(3, 2, msg='abc')`), the generated wrapper bound it correctly (`msg='abc'`) but then forwarded it positionally to `_call` (`_call(3, 2, 'abc')`). Any wrapper that inspects `*args` and `**kwargs` separately therefore lost the keyword information, producing the wrong result (mahmoud#343). Fix: arguments that carry a default in the original function are now forwarded with the `name=name` form (`return _call(x, y, msg=msg)`), so the wrapper receives them in `**kwargs` exactly as the caller passed them. As a side-effect, this also resolves the long-standing limitation noted in `test_wraps_inner_kwarg_only` (issue mahmoud#261): wrapping a keyword-only-`b` function with a signature that treats `b` as positional-or-keyword now works, because the generated invocation passes `b=b` which satisfies the keyword-only constraint. Tests that relied on extracting defaulted injected args from `*args` are updated to use `**kwargs` instead, which is the correct pattern under the new semantics.
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
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.
Fixes #343.
The bug
FunctionBuilder.get_invocation_str()passes every regular argumentpositionally in the generated body:
When a caller supplies a defaulted parameter by keyword (
pow(3, 2, msg='abc')),the generated function correctly binds
msg='abc', but then forwards itpositionally as
_call(3, 2, 'abc'). Any wrapper that inspects*args/**kwargsseparately—the standard pattern—loses the keywordinformation:
The fix
Args that carry a default are now forwarded with the
name=nameform:This means the inner wrapper receives them in
**kwargs, preservingexactly how the caller passed them.
As a welcome side-effect, this also lifts the limitation documented in
test_wraps_inner_kwarg_only(issue #261): wrapping a keyword-only-bfunction with a signature that exposes
bas positional-or-keyword nowworks, because the generated invocation passes
b=b.Test changes
test_wraps_preserves_kwargs_for_defaulted_argsintest_funcutils.pydirectly covering the issue wraps loses keywords #343 scenario.test_wraps_inner_kwarg_only: updated docstring and assertion toreflect that
wraps(g)(f)(3)now succeeds (was assertingTypeErrorwhich documented the old limitation).
test_wraps_expected(expect_pair/expect_dict): these wrappersextracted defaulted injected args from
args[-1], relying on the oldpositional-passthrough. Updated to use
kwargs.pop(...), which is thecorrect pattern under the new semantics.
test_get_invocation_sig_str: updated expected invocationstring for the single-defaulted-arg case (
"a"→"a=a").All 438 tests pass.