[kwwaleszczynski-del]

Closes #52.

AssessmentInputs.from_cfg reused preprocess.pickle whenever the file existed, so changes to the config (for example exclusion_periods_utc) or to the input data had no effect until the cache was deleted by hand.

Changes:

• with_pickle_cache takes an optional key. When it is given, the key is written to preprocess.pickle.key next to the pickle, and the cached result is only used if the saved key matches. The old key file is removed before the pickle is rewritten, so an interrupted write cannot leave a stale key behind.
• from_cfg passes a SHA-256 of the wind-up version, cfg.model_dump_json(), and the columns, column dtypes, index dtypes and pd.util.hash_pandas_object of the SCADA, metadata and reanalysis data. The dtypes are needed because hash_pandas_object gives the same hash for int and bool values and for a naive and a UTC index.
• Turbine latitude and longitude are left out of the config part (asset.wtgs, test_wtgs and ref_wtgs, which share their Turbine instances), because add_smart_lat_long_to_cfg copies them into cfg from the metadata after preprocessing. Otherwise a second from_cfg call with the same cfg object would miss the cache once.
• In an editable install the wind-up version does not change with the code, so changes to the preprocessing steps do not invalidate the cache there (noted in the docstring).
• plot_cfg is left out, since it only affects the plots. out_dir is part of the config, so changing it also invalidates the cache. That is on the safe side, but I can exclude it if you prefer.
• An existing preprocess.pickle without a key file is recomputed once.
• version is bumped to 0.5.1.

Hashing is small next to preprocessing: about 60 ms for the 8,928-row Homer test file, and, from the timings in the review, roughly 2 to 3 s for 3M rows with a UTC DatetimeIndex and about 5.5 s per 1M rows with a MultiIndex.

Tests: three new tests for the keyed pickle cache and two for the key (config, data, coordinates via add_smart_lat_long_to_cfg, dtypes and index time zone). I don't have the LFS test data locally, so I ran the suite without test_smarteole.py, test_wedowind.py and tests/plots/test_input_data.py: 189 passed. The two errors in TestCompareActiveAndReactivePowerPrePost also happen on main without the LFS files. ruff format --check, ruff check and mypy . pass. The smarteole and wedowind tests go through from_cfg with a cache_dir, so CI will exercise the new path on real data.

[Copilot]

🟡 Changes recommended

The cache key can reuse incorrectly aligned SCADA data when index timezone information changes, while shared turbine-coordinate mutations can also trigger unnecessary preprocessing reruns.

2 open issues

• Include datetime index timezone in the SCADA cache key · New
• Exclude shared turbine coordinates from the config key · New

What changed in this PR

This PR addresses #52 by making preprocessing cache reuse depend on configuration and input data, rather than file existence alone.

Changes:

• Adds optional pickle cache keys stored in sidecar files.
• Computes a preprocessing key from the wind-up version, configuration, and input data.
• Adds tests for keyed cache reuse, invalidation, and legacy caches without keys.

File	Description
wind_up/​interface.py	Computes and supplies the preprocessing cache key.
wind_up/​caching.py	Checks saved keys and invalidates them before rewriting cached data.
tests/​test_interface.py	Tests key stability and changes caused by configuration or data updates.
tests/​test_helpers.py	Tests cache hits, changed keys, and migration from unkeyed caches.

_{Review effort: Balanced}

[aclerc]

Thanks very much, Konrad! This is a good fix for #52 and the keyed pickle cache in caching.py is clean. I checked out the branch, ran the new tests (9 passed) and confirmed CI is green.

Must fix

1. The lat/long exclusion needs to cover test_wtgs and ref_wtgs too.
from_yaml builds test_wtgs and ref_wtgs from the same Turbine instances as asset.wtgs, so after add_smart_lat_long_to_cfg the coordinates also appear under those two lists in model_dump_json. I computed the key before and after the lat/long copy on the test_LSA_T13 config and it changes, so a second from_cfg call with the same cfg object still misses the cache once. Suggested:

coords = {"__all__": {"latitude", "longitude"}}
cfg_json = cfg.model_dump_json(exclude={"asset": {"wtgs": coords}, "test_wtgs": coords, "ref_wtgs": coords})

2. The regression test currently can't see this.
test_preprocess_cache_key_changes_with_inputs mutates asset.wtgs[0], which is LSA_T01, neither a test nor a ref turbine. Mutating test_lsa_t13_config.test_wtgs[0].latitude instead makes the last assertion fail on this branch, which is what the test should guard. Even better would be to call add_smart_lat_long_to_cfg on a small metadata frame so the test runs the real path.

3. Add dtypes and index dtype(s) to the hashed header.
hash_pandas_object gives identical hashes for a naive and a UTC-aware DatetimeIndex with the same stored values (Copilot's point: with a non-UTC metadata time zone that would reuse misaligned data), and also for int and bool columns with equal values. One line covers both and works for a MultiIndex:

index_dtypes = [str(lvl.dtype) for lvl in getattr(df.index, "levels", [df.index])]
h.update(f"{name}{list(df.columns)}{list(map(str, df.dtypes))}{index_dtypes}".encode())

Release

Could you please also bump version in pyproject.toml to 0.5.1 in this PR? That way I can publish a release as soon as it's merged.

Optional

• Hashing cost. The 60 ms figure is for the small Homer fixture. On a farm-sized frame I measured roughly 2 to 3 s for 3M rows with a UTC DatetimeIndex, and about 5.5 s per 1M rows with a MultiIndex. Still small next to preprocessing, but could update description.
• Editable installs. __version__ comes from package metadata, so code changes to the preprocessing steps don't invalidate the cache in a dev install. Pre-existing and outside #52, but a short note in the _preprocess_cache_key docstring would address.
• Keeping out_dir in the key is OK. Callers I know of keep out_dir fixed per assessment.
• Nit: caching.py wraps fp in Path() several times. Normalising once (fp = Path(fp)) and using fp.with_suffix(fp.suffix + ".key") is an option.
• Follow-up idea, not for this PR: the root cause of the exclusion is that add_smart_lat_long_to_cfg mutates the caller's cfg in place. Returning a copy would remove the special case altogether.

[kwwaleszczynski-del]

Thanks for the careful review, and for timing it on farm-sized frames.

Pushed b2ac803:

• The lat/long exclusion now covers asset.wtgs, test_wtgs and ref_wtgs, as you suggested.
• The regression test now builds a small metadata frame and calls add_smart_lat_long_to_cfg on the LSA_T13 config. It first checks that the coordinates of the test and ref turbines really changed, then that the key did not. It fails on the previous commit.
• The hashed header includes the column dtypes and the index level dtypes. A new test checks that the key changes for int vs bool values and for a naive vs UTC index; both gave the same key before.
• version is 0.5.1 in pyproject.toml and in the project entry of uv.lock.
• Added the editable-install note to the _preprocess_cache_key docstring, and with_pickle_cache now normalises fp once.
• Updated the description with your hashing timings.

Agreed that returning a copy from add_smart_lat_long_to_cfg would be the cleaner fix. I left it out of this PR.