Skip to content

Commit a0b7aff

Browse files
committed
memoize_source: pass the other arguments by name
joblib 1.3 (the declared floor) mistakes positional arguments beyond the fingerprint ones for the keyword-only `_source` parameter of the memoized function ("Keyword-only parameter '_source' was passed as positional parameter"), which broke `validate(path, readable=...)` under lowest-deps. Bind the decorated function's other arguments by name instead (defaults included), which also makes the cache key independent of how they were passed; the decorated function must not take *args as a consequence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFhMPs6vF5zyeHDvPTvRcA
1 parent ef659b8 commit a0b7aff

2 files changed

Lines changed: 27 additions & 4 deletions

File tree

‎dandi/pynwb_utils.py‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,20 @@ def memoize_source(cache: PersistentCache, tokens: Sequence[Any]) -> Callable[[F
9494
"""
9595

9696
def decorator(f: F) -> F:
97+
sig = inspect.signature(f)
98+
if any(p.kind is p.VAR_POSITIONAL for p in sig.parameters.values()):
99+
raise TypeError(f"memoize_source: {f.__qualname__}() must not take *args")
97100
by_path = cache.memoize_path(f)
98101

99102
def by_fingerprint(
100103
filename: str,
101104
fingerprint: str,
102105
cache_tokens: tuple[Any, ...],
103-
*args: Any,
106+
*,
104107
_source: Readable,
105108
**kwargs: Any,
106109
) -> Any:
107-
return f(_source, *args, **kwargs)
110+
return f(_source, **kwargs)
108111

109112
# Give it an identity of its own under `f`'s module, so that joblib
110113
# keeps its results in a per-function directory next to `by_path`'s
@@ -128,13 +131,23 @@ def wrapper(source: Any, *args: Any, **kwargs: Any) -> Any:
128131
fingerprint = source.get_fingerprint()
129132
if fingerprint is None:
130133
return f(source, *args, **kwargs)
134+
# Pass `f`'s other arguments by name (defaults included), so that the
135+
# cache key does not depend on how they were passed, and so that
136+
# joblib < 1.4 does not mistake positional ones for `_source`
137+
bound = sig.bind(source, *args, **kwargs)
138+
bound.apply_defaults()
139+
others: dict[str, Any] = {}
140+
for name, value in list(bound.arguments.items())[1:]:
141+
if sig.parameters[name].kind is inspect.Parameter.VAR_KEYWORD:
142+
others.update(value)
143+
else:
144+
others[name] = value
131145
return cached_by_fingerprint(
132146
source.get_filename(),
133147
fingerprint,
134148
tuple(tokens),
135-
*args,
136149
_source=source,
137-
**kwargs,
150+
**others,
138151
)
139152

140153
return cast(F, wrapper)

‎dandi/tests/test_pynwb_utils.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,9 @@ def size(source: str | Path | Readable, flag: bool = False) -> str:
287287
cached(FingerprintedReadable(simple1_nwb, "A"), flag=True) == f"{nbytes}:True"
288288
)
289289
assert len(calls) == 7
290+
# ... however those other arguments are passed
291+
assert cached(FingerprintedReadable(simple1_nwb, "A"), True) == f"{nbytes}:True"
292+
assert len(calls) == 7
290293
other_tokens = memoize_source(cache, ["t2"])(size)
291294
assert other_tokens(FingerprintedReadable(simple1_nwb, "A")) == expected
292295
assert len(calls) == 8
@@ -300,6 +303,13 @@ def name(source: str | Path | Readable, flag: bool = False) -> str:
300303
assert cached(FingerprintedReadable(simple1_nwb, "A")) == expected
301304
assert len(calls) == 8
302305

306+
# The other arguments are keyed by name, so `f` must not take *args
307+
def varargs(source: str | Path | Readable, *args: Any) -> None:
308+
pass
309+
310+
with pytest.raises(TypeError, match="must not take"):
311+
memoize_source(cache, ["t1"])(varargs)
312+
303313

304314
@pytest.mark.ai_generated
305315
def test_memoize_source_ignored_cache(

0 commit comments

Comments
 (0)