From 0d4d6eb8cc8ea32b5856ecc293e8781dc38a4b61 Mon Sep 17 00:00:00 2001 From: Piotr Synowiec Date: Tue, 25 Aug 2026 13:28:12 +0000 Subject: [PATCH 1/2] task(underscore_args): match st.cache_data for underscore-prefixed arguments An argument whose parameter name starts with an underscore was only kept out of the cache key when the caller passed it by keyword. _entry_key filtered kwargs by name and hashed the args tuple whole, and a tuple carries no names, so the same argument passed positionally still landed in the key. st.cache_data resolves positional arguments to their parameter names first and skips them either way, which is what the README promises parity with. Resolve positional slots to their parameter names through inspect.signature, once per decorated function rather than per call, and drop the underscore ones before hashing. Slots filled by *args keep being hashed: there is no name there to test. Functions with no introspectable signature fall back to hashing every positional argument. Keying by name rather than by position also means a positional call and a keyword call with the same values now reach one entry instead of two. Keyword order stays insensitive, which is a deliberate departure from st.cache_data: it hashes kwargs in caller order, so it treats f(a=1, b=2) and f(b=2, a=1) as separate entries. Sorting by name is the better behaviour and two existing tests already cover it. The key payload changed, so entries written by 0.1.0 are orphaned on upgrade. Ones with a TTL expire and get swept as usual; ttl=None entries are never swept and need clear(). Minor version bump for that. --- README.md | 31 +++++++++++++ pyproject.toml | 2 +- src/mysiar/disk_cache_data/__init__.py | 60 +++++++++++++++++++++++--- tests/test_clear.py | 24 +++++++++++ tests/test_disk_cache_data.py | 42 ++++++++++++++++++ 5 files changed, 152 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6eadbc7..2a0e364 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,37 @@ result = load_data(1, 2) result = load_data(1, 2) ``` +### Excluding an argument from the cache key + +An argument whose parameter name starts with an underscore is left out of the cache +key, as in `st.cache_data`. It still reaches the function on every computed call, but +changing it does not create a new entry. Use it for values the result does not depend +on, or for values that are expensive or impossible to hash. + +The name is resolved from the function signature, so the rule applies whether the +caller passes the argument positionally or by keyword. + +```python +@disk_cache_data(ttl="30s") +def load_data(a, _conn): + return _conn.query(a) + +load_data(1, conn_one) # computed and cached +load_data(1, conn_two) # hits the entry above; conn_two is not part of the key +``` + +Values absorbed by a `*args` parameter have no name to test, so they are always +hashed. + +Arguments are keyed by name rather than by position, so a positional call and a +keyword call with the same values reach the same entry, and keyword order never +matters: + +```python +load_data(1, conn) # computed and cached +load_data(a=1, _conn=conn) # hits the same entry +``` + ### Clearing the cache of one function Each decorated function stores its entries in its own subdirectory of the namespace, diff --git a/pyproject.toml b/pyproject.toml index db947d4..5393d57 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mysiar-disk-cache-data" -version = "0.1.0" +version = "0.2.0" description = "disk_cache_data decorator" authors = [{ name = "Piotr Synowiec " }] requires-python = ">=3.12" diff --git a/src/mysiar/disk_cache_data/__init__.py b/src/mysiar/disk_cache_data/__init__.py index 5c124ac..6f31bdb 100644 --- a/src/mysiar/disk_cache_data/__init__.py +++ b/src/mysiar/disk_cache_data/__init__.py @@ -1,5 +1,6 @@ import functools import hashlib +import inspect import os import pickle import re @@ -49,10 +50,53 @@ def function_dir(func, namespace=None) -> str: return os.path.join(_namespace_dir(namespace), function_dir_name(func)) -def _entry_key(args, kwargs) -> str: - """Hash of the call arguments. Kwargs are sorted so key order cannot change it.""" - filtered_kwargs = {k: kwargs[k] for k in sorted(kwargs) if not k.startswith("_")} - return hashlib.sha256(pickle.dumps((args, filtered_kwargs))).hexdigest() +def _positional_arg_names(func) -> tuple: + """Parameter name of each positional slot, None where that slot has no name. + + Slots filled by *args, and keyword-only parameters, have no positional name. + Values landing there are hashed as-is, since there is no name to test for the + underscore prefix. Same rule as st.cache_data. + """ + try: + params = inspect.signature(func).parameters.values() + except (TypeError, ValueError): + # Builtins and some C callables expose no signature. Without names, every + # positional argument is hashed, which is the safe direction. + return () + + nameable = (inspect.Parameter.POSITIONAL_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD) + return tuple(p.name if p.kind in nameable else None for p in params) + + +def _entry_key(positional_names, args, kwargs) -> str: + """Hash of the call arguments. + + An argument whose parameter name starts with an underscore is left out of the + key, as in st.cache_data: it still reaches the function, but changing it does + not mint a new entry. Positional arguments are resolved to their parameter + name first, so the exclusion holds whether the caller passes them + positionally or by keyword, and both call styles reach the same entry. + + Named arguments are sorted by name, so keyword order cannot change the key. + Values with no name keep their positional order, which is all that identifies + them. + """ + named = {} + unnamed = [] + + for index, value in enumerate(args): + name = positional_names[index] if index < len(positional_names) else None + if name is None: + unnamed.append(value) + elif not name.startswith("_"): + named[name] = value + + for name, value in kwargs.items(): + if not name.startswith("_"): + named[name] = value + + payload = ([(name, named[name]) for name in sorted(named)], unnamed) + return hashlib.sha256(pickle.dumps(payload)).hexdigest() def _mtime(path) -> float: @@ -99,6 +143,10 @@ def disk_cache_data(ttl=None): """ def decorator(func): + # Resolved once here rather than per call: inspect.signature is not cheap + # enough for a hot path, and the signature cannot change afterwards. + positional_names = _positional_arg_names(func) + @functools.wraps(func) def wrapper(*args, **kwargs): # ---- 0. Global disable ---- @@ -119,7 +167,7 @@ def wrapper(*args, **kwargs): effective_ttl = parse_ttl(ttl) # ---- 2. Locate entry inside the directory of this function ---- - key_hash = _entry_key(args, kwargs) + key_hash = _entry_key(positional_names, args, kwargs) fn_dir = function_dir(func) os.makedirs(fn_dir, exist_ok=True) @@ -187,7 +235,7 @@ def clear(*args, **kwargs): fn_dir = function_dir(func) if args or kwargs: - key_hash = _entry_key(args, kwargs) + key_hash = _entry_key(positional_names, args, kwargs) with _disk_lock: safe_delete(os.path.join(fn_dir, f"{key_hash}.pkl")) safe_delete(os.path.join(fn_dir, f"{key_hash}.meta")) diff --git a/tests/test_clear.py b/tests/test_clear.py index 22cf1a6..c674164 100644 --- a/tests/test_clear.py +++ b/tests/test_clear.py @@ -22,6 +22,12 @@ def multiply(a, b): return a * b +@disk_cache_data(ttl="10s") +def subtract(a, _token): + print("subtract-called") + return a - 1 + + class ClearTestCase(unittest.TestCase): """Per function cache clearing tests.""" @@ -90,6 +96,24 @@ def test_clear_with_arguments_removes_single_entry(self) -> None: self.assertEqual(add(3, 4), 7) self.assertEqual(self.__entries(add), 1) + def test_clear_ignores_private_args_whatever_the_call_style(self) -> None: + subtract(5, "token-used-at-call-time") + self.assertEqual(self.__entries(subtract), 1) + + # A private argument is not part of the key, so clear() finds the entry + # with any value for it, positionally or by keyword. + subtract.clear(5, "a-completely-different-token") + + self.assertEqual(self.__entries(subtract), 0) + + def test_clear_matches_an_entry_cached_with_the_other_call_style(self) -> None: + add(a=1, b=2) + self.assertEqual(self.__entries(add), 1) + + add.clear(1, 2) + + self.assertEqual(self.__entries(add), 0) + def test_clear_with_kwargs_ignores_keyword_order(self) -> None: add(a=1, b=2) self.assertEqual(self.__entries(add), 1) diff --git a/tests/test_disk_cache_data.py b/tests/test_disk_cache_data.py index 5a12661..d401c11 100644 --- a/tests/test_disk_cache_data.py +++ b/tests/test_disk_cache_data.py @@ -44,6 +44,18 @@ def load_data_private_kwargs(a, _token=None): return (a, _token) +@disk_cache_data(ttl="10s") +def load_data_private_positional(a, _token): + print("function-private-positional") + return (a, _token) + + +@disk_cache_data(ttl="10s") +def load_data_private_star_args(a, *rest): + print("function-private-star") + return (a, rest) + + def _decorated_from_module(module_name): """Build a decorated function whose qualified name is shared across modules.""" @@ -250,6 +262,36 @@ def test_private_kwargs_are_passed_to_function_but_kept_out_of_the_key(self, moc self.assertEqual(self.__calls(mock_stdout, "function-private"), 1) self.assertEqual(self.__entries(load_data_private_kwargs), 1) + @patch("sys.stdout", new_callable=io.StringIO) + def test_private_args_are_kept_out_of_the_key_when_passed_positionally(self, mock_stdout) -> None: + first = load_data_private_positional(1, "first") + second = load_data_private_positional(1, "second") + + # The name is resolved from the signature, so the underscore rule applies + # to a positional argument exactly as it does to a keyword one. + self.assertEqual(first, (1, "first")) + self.assertEqual(second, (1, "first")) + self.assertEqual(self.__calls(mock_stdout, "function-private-positional"), 1) + self.assertEqual(self.__entries(load_data_private_positional), 1) + + @patch("sys.stdout", new_callable=io.StringIO) + def test_positional_and_keyword_calls_share_one_entry(self, mock_stdout) -> None: + self.assertEqual(load_data(1, 2), 3) + self.assertEqual(load_data(a=1, b=2), 3) + self.assertEqual(load_data(1, b=2), 3) + + self.assertEqual(self.__calls(mock_stdout, "function"), 1) + self.assertEqual(self.__entries(load_data), 1) + + @patch("sys.stdout", new_callable=io.StringIO) + def test_values_absorbed_by_star_args_stay_in_the_key(self, mock_stdout) -> None: + # A *args slot has no parameter name to test, so its values are hashed. + self.assertEqual(load_data_private_star_args(1, "x"), (1, ("x",))) + self.assertEqual(load_data_private_star_args(1, "y"), (1, ("y",))) + + self.assertEqual(self.__calls(mock_stdout, "function-private-star"), 2) + self.assertEqual(self.__entries(load_data_private_star_args), 2) + @patch("sys.stdout", new_callable=io.StringIO) def test_expired_entry_is_recomputed_and_replaced(self, mock_stdout) -> None: load_data(1, 2) From 03f0e23d8063cee118e82712c30896afc45225a1 Mon Sep 17 00:00:00 2001 From: Piotr Synowiec Date: Tue, 25 Aug 2026 13:36:16 +0000 Subject: [PATCH 2/2] task(ci): add GitHub Actions workflow for unit tests Runs ruff and the pytest suite with coverage on every branch push, on pull requests and on manual dispatch, across Python 3.12 and 3.13. Mirrors the lint and tests targets in the Makefile. Concurrency group keeps one run per ref, cancelling the run still in flight. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/tests.yml | 42 +++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..d54780f --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,42 @@ +name: tests + +on: + push: + pull_request: + workflow_dispatch: + +# One run per ref: a new push cancels the run still in flight for the same branch. +concurrency: + group: tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + tests: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.12", "3.13"] + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: pip + cache-dependency-path: pyproject.toml + + - name: Install package with dev extras + run: | + python -m pip install --upgrade pip + pip install -e .[dev] + + - name: Lint + run: ruff check src tests + + - name: Tests + # DISK_CACHE_DIR is read at import time, so the env must be set before pytest starts. + run: | + source tests/env.tests.sh + pytest -vrP tests --cov=src/mysiar/disk_cache_data --cov-report=term-missing