Skip to content

Commit 02ba00e

Browse files
committed
Use fscacher's memoize_path(content_fingerprint=...) instead of memoize_source
The fingerprint-keyed caching is being upstreamed to fscacher (con/fscacher#113), where it fits into memoize_path itself: the source argument is already excluded from joblib's key there, and the cache's tokens are at hand. That makes most of `memoize_source` unnecessary: the separately named `by_fingerprint` function, passing tokens explicitly (and the tokens refactor it required), the `exclude_kwargs`/`ignore` shim, and binding the other arguments by name to dodge joblib < 1.4. What remains is `readable_fingerprint()`, which returns (file name, fingerprint) for a `Readable` that knows its fingerprint and `None` otherwise, passed as `content_fingerprint` at the three call sites. This also restores caching for fingerprint-less `LocalReadableFile`s: being path-like, memoize_path has always cached them by their stat(), but `memoize_source` routed every `Readable` away from it. fscacher is pinned to the PR branch for now; to be replaced with a version floor once it is released. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qn5WBSiQgoZoL4fytF6nEr
1 parent 9b6930e commit 02ba00e

5 files changed

Lines changed: 57 additions & 157 deletions

File tree

‎dandi/metadata/nwb.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,17 @@
1919
get_neurodata_types,
2020
get_nwb_version,
2121
ignore_benign_pynwb_warnings,
22-
memoize_source,
2322
metadata_cache,
24-
metadata_cache_tokens,
2523
nwb_has_external_links,
24+
readable_fingerprint,
2625
)
2726
from ..utils import find_parent_directory_containing
2827

2928
lgr = get_logger()
3029

3130

3231
# Disable this for clean hacking
33-
@memoize_source(metadata_cache, metadata_cache_tokens)
32+
@metadata_cache.memoize_path(content_fingerprint=readable_fingerprint)
3433
def get_metadata(
3534
path: str | Path | Readable, digest: Digest | None = None
3635
) -> dict[str, Any]:

‎dandi/misctypes.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,9 @@ def get_fingerprint(self) -> str | None:
294294
Two resources with equal fingerprints must have identical bytes: the
295295
fingerprint is what lets results derived from the content (metadata,
296296
validation results) be cached and reused across instances, see
297-
`dandi.pynwb_utils.memoize_source`. With the default of `None`, such
298-
results are never cached for the resource.
297+
`dandi.pynwb_utils.readable_fingerprint`. With the default of `None`, such
298+
results are only cached for a path-like resource (by the ``stat()`` of
299+
its path), never for others.
299300
"""
300301
return None
301302

‎dandi/pynwb_utils.py‎

Lines changed: 20 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,8 @@
1212
from __future__ import annotations
1313

1414
from collections import Counter
15-
from collections.abc import Callable, Sequence
15+
from collections.abc import Callable
1616
from datetime import timedelta
17-
from functools import wraps
1817
import inspect
1918
import os
2019
import os.path as op
@@ -64,95 +63,31 @@
6463
get_module_version(hdmf),
6564
get_module_version(h5py),
6665
]
67-
metadata_cache_tokens = dandi_cache_tokens
6866
metadata_cache = PersistentCache(
69-
name="dandi-metadata", tokens=metadata_cache_tokens, envvar="DANDI_CACHE"
67+
name="dandi-metadata", tokens=dandi_cache_tokens, envvar="DANDI_CACHE"
7068
)
71-
validate_cache_tokens = dandi_cache_tokens + [get_module_version(dandischema)]
7269
validate_cache = PersistentCache(
73-
name="dandi-validate", tokens=validate_cache_tokens, envvar="DANDI_CACHE"
70+
name="dandi-validate",
71+
tokens=dandi_cache_tokens + [get_module_version(dandischema)],
72+
envvar="DANDI_CACHE",
7473
)
7574

76-
F = TypeVar("F", bound=Callable[..., Any])
7775

78-
79-
def memoize_source(cache: PersistentCache, tokens: Sequence[Any]) -> Callable[[F], F]:
76+
def readable_fingerprint(source: Any) -> tuple[str, str] | None:
8077
"""
81-
Decorator caching the results of a function of a local path or a `Readable`
82-
83-
For a path argument (`str` or `Path`), this is exactly ``cache.memoize_path``:
84-
the result is cached under the file's location, a fingerprint of the file
85-
(mtime, ctime, size, ...), and ``tokens``. A `Readable` cannot be
86-
fingerprinted that way, so `PersistentCache.memoize_path` leaves calls on
87-
one uncached; here, a `Readable` whose `~Readable.get_fingerprint` returns
88-
a value is cached under its file name, that fingerprint, and ``tokens``
89-
instead. A `Readable` without a fingerprint is still never cached.
90-
91-
``tokens`` should be the ones ``cache`` was created with (versions of the
92-
software producing the results): `PersistentCache.memoize`, unlike
93-
`~PersistentCache.memoize_path`, does not add them by itself.
78+
Content fingerprint of ``source`` for ``PersistentCache.memoize_path``
79+
80+
Pass it as ``content_fingerprint`` to cache the results of a function of a
81+
local path or a `Readable`. Paths keep being cached under their location
82+
and ``stat()``; a `Readable` cannot be fingerprinted that way, so one whose
83+
`~Readable.get_fingerprint` returns a value is cached under its file name
84+
and that fingerprint instead. A `Readable` without a fingerprint is handled
85+
as without this: cached by its path if it is path-like (as
86+
`LocalReadableFile` is), not cached at all otherwise.
9487
"""
95-
96-
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")
100-
by_path = cache.memoize_path(f)
101-
102-
def by_fingerprint(
103-
filename: str,
104-
fingerprint: str,
105-
cache_tokens: tuple[Any, ...],
106-
*,
107-
_source: Readable,
108-
**kwargs: Any,
109-
) -> Any:
110-
return f(_source, **kwargs)
111-
112-
# Give it an identity of its own under `f`'s module, so that joblib
113-
# keeps its results in a per-function directory next to `by_path`'s
114-
by_fingerprint.__module__ = f.__module__
115-
by_fingerprint.__name__ = f"{f.__name__}__by_fingerprint"
116-
by_fingerprint.__qualname__ = f"{f.__qualname__}__by_fingerprint"
117-
# fscacher >= 0.4 calls the argument `exclude_kwargs`, older ones `ignore`
118-
ignore_kwarg = (
119-
"exclude_kwargs"
120-
if "exclude_kwargs" in inspect.signature(cache.memoize).parameters
121-
else "ignore"
122-
)
123-
cached_by_fingerprint = cache.memoize(
124-
by_fingerprint, **{ignore_kwarg: ["_source"]}
125-
)
126-
127-
@wraps(f)
128-
def wrapper(source: Any, *args: Any, **kwargs: Any) -> Any:
129-
if not isinstance(source, Readable):
130-
return by_path(source, *args, **kwargs)
131-
fingerprint = source.get_fingerprint()
132-
if fingerprint is None:
133-
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
145-
return cached_by_fingerprint(
146-
source.get_filename(),
147-
fingerprint,
148-
tuple(tokens),
149-
_source=source,
150-
**others,
151-
)
152-
153-
return cast(F, wrapper)
154-
155-
return decorator
88+
if isinstance(source, Readable) and (fp := source.get_fingerprint()) is not None:
89+
return (source.get_filename(), fp)
90+
return None
15691

15792

15893
def _sanitize_nwb_version(
@@ -276,7 +211,7 @@ def get_neurodata_types_to_modalities_map() -> dict[str, str]:
276211
return ndtypes
277212

278213

279-
@memoize_source(metadata_cache, metadata_cache_tokens)
214+
@metadata_cache.memoize_path(content_fingerprint=readable_fingerprint)
280215
def get_neurodata_types(filepath: str | Path | Readable) -> list[str]:
281216
with open_readable(filepath) as fp, h5py.File(fp, "r") as h5file:
282217
all_pairs = _scan_neurodata_types(h5file)
@@ -890,7 +825,7 @@ def copy_nwb_file(src: str | Path, dest: str | Path) -> str:
890825
return str(dest)
891826

892827

893-
@memoize_source(metadata_cache, metadata_cache_tokens)
828+
@metadata_cache.memoize_path(content_fingerprint=readable_fingerprint)
894829
def nwb_has_external_links(filepath: str | Path | Readable) -> bool:
895830
with open_readable(filepath) as f, h5py.File(f, "r") as fp:
896831
visited = set()

‎dandi/tests/test_pynwb_utils.py‎

Lines changed: 29 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
from ..pynwb_utils import (
2323
_rename_pose_estimation_original_videos,
2424
_sanitize_nwb_version,
25-
memoize_source,
2625
nwb_has_external_links,
2726
open_readable,
27+
readable_fingerprint,
2828
rename_nwb_external_files,
2929
)
3030

@@ -241,93 +241,56 @@ def test_nwb_has_external_links(tmp_path):
241241

242242

243243
@pytest.mark.ai_generated
244-
def test_memoize_source(tmp_path: Path, simple1_nwb: Path) -> None:
244+
def test_readable_fingerprint(tmp_path: Path, simple1_nwb: Path) -> None:
245+
assert readable_fingerprint(simple1_nwb) is None
246+
assert readable_fingerprint(str(simple1_nwb)) is None
247+
assert readable_fingerprint(FingerprintedReadable(simple1_nwb, None)) is None
248+
assert readable_fingerprint(FingerprintedReadable(simple1_nwb, "A")) == (
249+
simple1_nwb.name,
250+
"A",
251+
)
252+
253+
254+
@pytest.mark.ai_generated
255+
def test_memoize_path_readable_fingerprint(tmp_path: Path, simple1_nwb: Path) -> None:
245256
cache = PersistentCache(path=tmp_path / "cache", tokens=["t1"])
246257
calls: list[Any] = []
247258

259+
@cache.memoize_path(content_fingerprint=readable_fingerprint)
248260
def size(source: str | Path | Readable, flag: bool = False) -> str:
249261
calls.append(source)
250262
with open_readable(source) as fp:
251263
return f"{len(fp.read())}:{flag}"
252264

253-
cached = memoize_source(cache, ["t1"])(size)
254-
nbytes = simple1_nwb.stat().st_size
255-
expected = f"{nbytes}:False"
265+
expected = f"{simple1_nwb.stat().st_size}:False"
256266

257-
# A path is cached the memoize_path way (which skips a file modified "just
267+
# A path is still cached by its stat() (which skips a file modified "just
258268
# now", as the session-wide fixture may well have been: age a copy)
259269
nwb = tmp_path / simple1_nwb.name
260270
shutil.copyfile(simple1_nwb, nwb)
261271
hour_ago = time.time() - 3600
262272
os.utime(nwb, (hour_ago, hour_ago))
263-
assert cached(nwb) == expected
264-
assert cached(nwb) == expected
273+
assert size(nwb) == expected
274+
assert size(nwb) == expected
265275
assert len(calls) == 1
266276

267-
# A Readable without a fingerprint is never cached
268-
assert cached(FingerprintedReadable(simple1_nwb, None)) == expected
269-
assert cached(FingerprintedReadable(simple1_nwb, None)) == expected
270-
assert len(calls) == 3
277+
# A Readable without a fingerprint is handled as before: a local one is
278+
# path-like, so it is cached by the stat() of its path, sharing the entry
279+
local = FingerprintedReadable(nwb, None)
280+
assert size(local) == expected
281+
assert local.opened == 0
282+
assert len(calls) == 1
271283

272284
# A Readable with a fingerprint is cached by it: its twin is served from
273285
# the cache without being read (it could not be)
274286
first = FingerprintedReadable(simple1_nwb, "A")
275-
assert cached(first) == expected
287+
assert size(first) == expected
276288
assert first.opened == 1
277289
twin = FingerprintedReadable(tmp_path / "gone" / simple1_nwb.name, "A")
278-
assert cached(twin) == expected
290+
assert size(twin) == expected
279291
assert twin.opened == 0
280-
assert len(calls) == 4
292+
assert len(calls) == 2
281293

282-
# The file name, the other arguments, and the tokens are part of the key
294+
# ... but the file name is part of the key
283295
with pytest.raises(FileNotFoundError):
284-
cached(FingerprintedReadable(tmp_path / "gone" / "other.nwb", "A"))
285-
assert cached(FingerprintedReadable(simple1_nwb, "B")) == expected
286-
assert (
287-
cached(FingerprintedReadable(simple1_nwb, "A"), flag=True) == f"{nbytes}:True"
288-
)
289-
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
293-
other_tokens = memoize_source(cache, ["t2"])(size)
294-
assert other_tokens(FingerprintedReadable(simple1_nwb, "A")) == expected
295-
assert len(calls) == 8
296-
297-
# Different functions of the same cache do not share entries
298-
def name(source: str | Path | Readable, flag: bool = False) -> str:
299-
return source.get_filename() if isinstance(source, Readable) else str(source)
300-
301-
cached_name = memoize_source(cache, ["t1"])(name)
302-
assert cached_name(FingerprintedReadable(simple1_nwb, "A")) == simple1_nwb.name
303-
assert cached(FingerprintedReadable(simple1_nwb, "A")) == expected
304-
assert len(calls) == 8
305-
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-
313-
314-
@pytest.mark.ai_generated
315-
def test_memoize_source_ignored_cache(
316-
tmp_path: Path, simple1_nwb: Path, monkeypatch: pytest.MonkeyPatch
317-
) -> None:
318-
monkeypatch.setenv("DANDI_TEST_CACHE", "ignore")
319-
cache = PersistentCache(
320-
path=tmp_path / "cache", tokens=["t1"], envvar="DANDI_TEST_CACHE"
321-
)
322-
calls = 0
323-
324-
def count(source: str | Path | Readable) -> int:
325-
nonlocal calls
326-
calls += 1
327-
return calls
328-
329-
cached = memoize_source(cache, ["t1"])(count)
330-
assert cached(FingerprintedReadable(simple1_nwb, "A")) == 1
331-
assert cached(FingerprintedReadable(simple1_nwb, "A")) == 2
332-
assert cached(simple1_nwb) == 3
333-
assert cached(simple1_nwb) == 4
296+
size(FingerprintedReadable(tmp_path / "gone" / "other.nwb", "A"))

‎pyproject.toml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ dependencies = [
5353
"dandischema >= 0.12.0, != 0.13.0, < 0.15.0",
5454
"etelemetry >= 0.2.2",
5555
"fasteners >= 0.19",
56-
"fscacher >= 0.3.0",
56+
# TEMPORARY until con/fscacher#113 (memoize_path(content_fingerprint=...))
57+
# is released: then replace with "fscacher >= <that release>"
58+
"fscacher @ git+https://github.com/con/fscacher@claude/content-fingerprint",
5759
# Floor raised to what `py3-lowest` actually resolves: modern pynwb pins
5860
# hdmf >= 4, so declared floor is now the tested floor.
5961
"hdmf >= 4.1.0",

0 commit comments

Comments
 (0)