Skip to content

Commit a683275

Browse files
CopilotBordapre-commit-ci[bot]
authored
Fix cache key generation for variadic arguments (*args, **kwargs) (#322)
* Initial plan * Fix: Handle variadic arguments (*args, **kwargs) in cache key generation - Modified _convert_args_kwargs() to properly handle VAR_POSITIONAL and VAR_KEYWORD parameters - Variadic positional args are now expanded as individual entries with __varargs_N__ keys - Variadic keyword args are stored with their original keys - Added comprehensive test suite for variadic arguments - All existing tests pass including custom hash function tests * Address PR review feedback: keyword-only params, parametrize tests, simplify code - Fixed incorrect handling of keyword-only parameters (KEYWORD_ONLY should not be in regular_params) - Simplified VAR_KEYWORD handling by removing unreachable else branch - Parametrized first test to run on both pickle and memory backends - Added 3 new tests for keyword-only parameter edge cases - Fixed line length issues in docstrings and comments * Improve test assertions to explicitly verify cache hits - Changed assertions to capture previous call_count before testing cache hits - Makes tests more explicit: verify count didn't increase rather than re-asserting same value - Applied to test_varargs_different_cache_keys, test_varargs_empty, test_varkwargs_different_cache_keys, and test_varargs_and_varkwargs * Simplify parameter handling in `map_arguments` * Fix test assertion in test_varargs_memory_backend - Changed assertion to capture previous_call_count before cache hit test - Makes test explicit about verifying the count didn't increase - Consistent with other test improvements in previous commit * Refactor and parametrize varargs/varkwargs test cases for enhanced readability and backend coverage * Remove redundant `@pytest.mark.pickle` annotations from varargs/varkwargs test cases * [pre-commit.ci] auto fixes from pre-commit.com hooks * Add comprehensive edge case tests for variadic arguments - Added TestFunctoolsPartial: tests for functools.partial wrapped functions - Added TestMethodWithVarargs: tests for functions simulating method behavior - Added TestPositionalOnlyParams: tests for positional-only parameters (/) with varargs - Added TestComplexParameterMix: tests for functions with all parameter types combined - Added TestEdgeCasesEmptyAndNone: tests for None, empty strings, and zero values in varargs - Total test count increased from 42 to 68 tests (26 new tests) - All tests parametrized across pickle and memory backends for comprehensive coverage * Simplify redundant conditional in _convert_args_kwargs - Removed redundant if-else where both branches performed the same action - Simplified kwds handling to always use kwargs.update(kwds) - Removed unused var_keyword_name variable - All 68 tests still pass --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Borda <6035284+Borda@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 425287d commit a683275

2 files changed

Lines changed: 711 additions & 8 deletions

File tree

src/cachier/core.py

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,53 @@ def _convert_args_kwargs(func, _is_method: bool, args: tuple, kwds: dict) -> dic
7575
args = func.args + args
7676
kwds.update({k: v for k, v in func.keywords.items() if k not in kwds})
7777
func = func.func
78-
func_params = list(inspect.signature(func).parameters)
79-
args_as_kw = dict(zip(func_params[1:], args[1:]) if _is_method else zip(func_params, args))
80-
# init with default values
81-
kwargs = {
82-
k: v.default for k, v in inspect.signature(func).parameters.items() if v.default is not inspect.Parameter.empty
83-
}
84-
# merge args expanded as kwargs and the original kwds
85-
kwargs.update(dict(**args_as_kw, **kwds))
78+
79+
sig = inspect.signature(func)
80+
func_params = list(sig.parameters)
81+
82+
# Separate regular parameters from VAR_POSITIONAL
83+
regular_params = []
84+
var_positional_name = None
85+
86+
for param_name in func_params:
87+
param = sig.parameters[param_name]
88+
if param.kind == inspect.Parameter.VAR_POSITIONAL:
89+
var_positional_name = param_name
90+
elif param.kind in (
91+
inspect.Parameter.POSITIONAL_ONLY,
92+
inspect.Parameter.POSITIONAL_OR_KEYWORD,
93+
):
94+
regular_params.append(param_name)
95+
96+
# Map positional arguments to regular parameters
97+
if _is_method:
98+
# Skip 'self' for methods
99+
args_to_map = args[1:]
100+
params_to_use = regular_params[1:]
101+
else:
102+
args_to_map = args
103+
params_to_use = regular_params
104+
105+
# Map as many args as possible to regular parameters
106+
num_regular = len(params_to_use)
107+
args_as_kw = dict(zip(params_to_use, args_to_map[:num_regular]))
108+
109+
# Handle variadic positional arguments
110+
# Store them with indexed keys like __varargs_0__, __varargs_1__, etc.
111+
if var_positional_name and len(args_to_map) > num_regular:
112+
var_args = args_to_map[num_regular:]
113+
for i, arg in enumerate(var_args):
114+
args_as_kw[f"__varargs_{i}__"] = arg
115+
116+
# Init with default values
117+
kwargs = {k: v.default for k, v in sig.parameters.items() if v.default is not inspect.Parameter.empty}
118+
119+
# Merge args expanded as kwargs and the original kwds
120+
kwargs.update(args_as_kw)
121+
122+
# Handle keyword arguments (including variadic keyword arguments)
123+
kwargs.update(kwds)
124+
86125
return OrderedDict(sorted(kwargs.items()))
87126

88127

0 commit comments

Comments
 (0)