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 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)