Release/v1 - #105
Draft
crvernon wants to merge 14 commits into
Draft
Conversation
Adds plans/ with three planning documents produced from an audit of the package: - development-plan.md: current-state assessment, confirmed defects, 8 workstreams (baseline/regression, packaging, python matrix, correctness, code quality, performance, repo size, docs) and sequencing. - repo-clone-performance.md: diagnosis of slow clones (notebook base64 outputs, historical package-data blobs, duplicated binary assets) with measurement commands and both non-destructive and history-rewrite fixes. - benchmarks-and-regression-testing.md: golden-output invariance harness and pytest-benchmark suite design to guarantee refactors do not alter scientific outputs.
Adds a measurements section with concrete numbers from branch release/v1: - .git is 288 MB (274 MiB pack) against a ~2 MB working tree - 784.1 MB of uncompressed blobs across all history - 722.6 MB (92%) belongs to paths that no longer exist in HEAD Largest contributors are deleted stitched NetCDF outputs under notebooks/quickstart-ncs (188 MB), historical stitches/data package data (346 MB), and rendered dev notebooks under notebooks/stitches_dev (127 MB). notebooks/stitches-quickstart.ipynb has accumulated 38.5 MB over 31 revisions because outputs are committed. This evidence justifies the history rewrite option, so the filter-repo command was updated with the confirmed offending paths plus a verification step asserting HEAD is unchanged.
The suite had two structural problems: 1. tests/test_pangeo.py and tests/test_stitch.py gated their real assertions behind a hardcoded 'RUN = "ci"' class attribute whose false branch ran 'self.assertEqual(0, 0)'. In CI these tests reported as PASSING while exercising no code at all, so the gridded stitching and Pangeo code paths were effectively untested and any regression there was invisible. 2. tests/conftest.py installed the full Zenodo package data in a session-scoped autouse fixture, so every pytest invocation -- even a single pure unit test -- downloaded hundreds of megabytes. Changes: - Add pytest.ini registering markers (network, slow, package_data, regression, benchmark) with --strict-markers so typos fail loudly. - Rewrite tests/conftest.py: package data becomes an explicit opt-in fixture instead of autouse; add --network/--slow/--package-data flags with matching STITCHES_TEST_* env vars; add pytest_collection_modifyitems so gated tests skip with a visible reason rather than passing vacuously. Also adds the --update-golden flag used by the forthcoming regression suite, plus read_example/example_data_dir fixtures for the offline tier. - Convert both test modules from unittest.TestCase to pytest functions, split the monolithic test methods into single-behavior tests, and mark them by the capability they actually need. gmat_stitching is package_data; gridded stitching and Pangeo are network+slow. Gridded tests now write to tmp_path instead of the repo root and close datasets via context managers. - Strengthen a few assertions along the way: the row-order-invariance test now compares full frames rather than two scalar cells, and the Pangeo table test checks for required columns. - gitignore .venv/, .vscode/, and .benchmarks/. The offline tier now runs in ~2s with 15 passed / 13 visibly skipped, where previously it required a large download to run at all.
Reproducibility was previously only reachable through permute_stitching_recipes(testing=True), which hardcoded random_state=1. Overloading a 'testing' flag to mean 'be deterministic' is awkward for users who legitimately need reproducible recipes, and it also makes the golden-output regression suite depend on a flag whose name implies other behavior. Adds an optional 'seed' parameter to: - shuffle_function(dt, seed=None) - permute_stitching_recipes(..., seed=None) - make_recipe(..., seed=None) Backward compatibility is exact and deliberate: - seed=None with testing=False -> random_state=None, i.e. identical to the previous unseeded call, so existing nondeterministic behavior is unchanged. - seed=None with testing=True -> random_state=1, the previous testing branch. - an explicit seed takes precedence over testing. The seed is applied per groupby group rather than to one shared generator. This is intentional: it reproduces the original random_state=1 semantics bit for bit. Using a single shared generator would be cleaner but would change published outputs, which is out of scope here. Also collapses the duplicated if/else in make_recipe that called permute_stitching_recipes twice with only the testing flag differing. Adds tests/test_seed.py (13 tests) pinning the compatibility contract, including test_permute_seed_matches_legacy_testing_flag which asserts seed=1 and testing=True produce identical frames. No outputs change.
…coverage Establishes the safety net required before any refactor, dependency bump, or performance work can be trusted not to change scientific output. Harness (tests/regression/conftest.py): - GoldenComparer with assert_frame (Parquet artifacts), assert_digest (SHA-256 for artifacts too large to vendor), and assert_values (JSON for filenames and summary stats). - canonicalize() sorts columns by name and rows by full content, making the comparison insensitive to row/column ORDER but fully sensitive to CONTENT. This is deliberate: groupby/concat-heavy code legitimately reorders across pandas versions, and a suite that cries wolf on every such change gets ignored. Where order is part of the contract it is asserted explicitly. - Named tolerance presets rather than one global epsilon: exact (0) for indices, years and labels; 1e-12 for match distances; 1e-10 for temperature values; 1e-6/1e-9 for gridded fields. Integral and categorical data must be exact; float reductions get a tight but nonzero tolerance because NumPy and pandas may reassociate between versions without any change in meaning. - Artifacts are Parquet, not CSV, so float formatting cannot mask a real diff or manufacture a fake one. - --update-golden regenerates artifacts and prints what it rewrote along with a reminder to document the change. Coverage (tests/regression/test_invariance_match.py, 14 tests): a tolerance sweep over match_neighborhood in both dedup modes, self-matching, row counts, internal_dist, drop_hist_false_duplicates (whose min-idvalue tie-break is order-sensitive), and seeded shuffle_function. Validated by mutation testing rather than assumed to work: perturbing dist_l2 by one part in 1e9 produced 3 failures with a precise column-and-index diff, and the suite returned to green once reverted. All fixtures are the committed example CSVs, so the suite is fully offline and runs in ~2.5s.
Adding golden-output regression coverage for fx_processing immediately surfaced two hard failures that make the package unusable on a current scientific Python stack (tested against pandas 3.0.5 / numpy 2.5.2 / scikit-learn 1.9.0). 1. get_chunk_info raised TypeError: only 0-dimensional arrays can be converted to Python scalars. LinearRegression is fitted against a column vector, so coef_ has shape (1, 1) and coef_[0] is a one-element ARRAY, not a scalar. float() on a one-element array was deprecated in NumPy 1.25 and raises in NumPy 2. Since get_chunk_info computes the dx (rate of change) values that the entire matching algorithm keys on, this broke archive generation and recipe creation outright. Fixed with float(model.coef_.ravel()[0]), which selects the single coefficient explicitly. Verified value-identical to the legacy float(coef_[0]) semantics across 200 randomized fits, so no output changes. 2. calculate_rolling_mean raised ValueError: Cannot specify both 'axis' and 'index'/'columns'. drop(columns='value', axis=1) passes both selectors; pandas 3.0 rejects that combination. The axis=1 was redundant because columns= already implies the column axis, so it was removed with no behavioral change. Both are forward-compatibility defects rather than logic errors: the intended results were always well defined, the code simply relied on deprecated coercions. Also adds tests/regression/test_invariance_processing.py (20 tests) covering calculate_rolling_mean across window sizes, chunk_ts including staggered base_chunk offsets, get_chunk_info fx/dx values, and subset_archive, using a deterministic seeded synthetic series so window arithmetic is exercised at known lengths. Includes property assertions that do not depend on recorded values, such as min_periods=1 leaving no NaN at the series edges and each chunk's representative year lying within its own bounds. Confirmed the previously committed fx_match golden artifacts are byte-identical after these fixes (git diff on tests/regression/golden is empty), so the changes are provably side-effect free. Adds CHANGELOG.md, including the output-change policy requiring any golden artifact update to name the defect it corrects.
…rage Extends the golden-output suite to the remaining pure-Python core, bringing the offline suite to 98 passing tests and 34 golden artifacts (384K total). fx_recipe (21 tests) -- the most algorithmically involved module, and the one where an accidental change is hardest to notice: - get_num_perms targets and guide frames, which determine how many collapse-free realizations each target supports and therefore the order the whole construction proceeds in. - remove_duplicates, pinning which target year wins a contested archive point and what the loser is re-matched to. - permute_stitching_recipes at N=1 and N=2 and at two tolerances; the wider tolerance gives more candidates per window and exercises the sampling and duplicate-rejection loop far harder. - handle_transition_periods and handle_final_period, which rewrite the windows straddling the historical/future boundary. Beyond recorded values, this module asserts structural properties that hold for any valid draw: recipes cover every target window, draw only from the archive, never reuse an archive point within a realization, and exhibit no envelope collapse across realizations. It also asserts target coverage stays contiguous after transition handling, since a gap there would silently yield a stitched series with missing years. fx_util + fx_data (15 tests): - selstr, nrow, combine_df, and anti_join, including the cross-join row count and the shared-column rejection. - global_mean, which applies the cosine-of-latitude area weighting. Verified analytically as well as against a golden: a spatially uniform field must average back to its own constant (catching a normalization bug), and a cos(lat) field must yield a weighted mean strictly greater than the unweighted mean (catching weighting being skipped entirely). This matters because a weighting error would bias every emulated series while still producing plausible numbers. - get_lat_name under both the 'lat' and 'latitude' spellings. While writing these, remove_duplicates was found to require singular matches and to raise TypeError otherwise; that precondition is now covered by an explicit test rather than left implicit. Confirmed no previously committed golden artifact changed.
Adds 33 benchmarks over fx_match, fx_processing, and fx_recipe, parametrized by
input size so they reveal SCALING rather than a single opaque number. Scaling is
what identifies worthwhile optimization targets; absolute timings on a shared
runner do not.
Findings from the recorded baseline, now documented in
plans/benchmarks-and-regression-testing.md section 0:
- match_neighborhood is the top target. It is both the most expensive path and
slightly superlinear in target windows: 28 -> 112 -> 280 windows costs
59.5ms -> 234.7ms -> 646.5ms, i.e. 10.9x cost for 10x input.
- The bottleneck is iterating target groups, not searching the archive. Widening
the archive 8x (2 -> 16 ensemble members) costs only 1.6x. Optimization should
therefore attack the per-group Python loop, not the lookup.
- calculate_rolling_mean is dominated by GROUP count, not row count, at roughly
450-700us fixed overhead per group, and is entirely insensitive to window size
(3/9/21 all ~9.65ms). That points squarely at the groupby.transform lambda.
- get_chunk_info costs ~700us per chunk, spent building a scikit-learn
LinearRegression per chunk and growing the frame by repeated pd.concat. A
closed-form slope would remove most of it, and the golden artifacts committed
earlier make that safe to attempt.
- permute_stitching_recipes saturates in N_matches on the synthetic archive
(N=2 and N=5 are indistinguishable because the archive cannot support five
collapse-free realizations) but scales cleanly with tolerance. Noted so the
saturation is not later mistaken for an optimization win.
Benchmarks are excluded from the default pytest run via testpaths, and
bench_*.py was added to python_files so ============================= test session starts ==============================
platform darwin -- Python 3.13.3, pytest-8.3.5, pluggy-1.6.0
rootdir: /Users/d3y010/repos/github/stitches
configfile: pytest.ini
plugins: anyio-4.9.0, asyncio-1.3.0, logfire-4.21.0, hypothesis-6.152.9, langsmith-0.3.33
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
collected 0 items / 3 errors
==================================== ERRORS ====================================
__________________ ERROR collecting benchmarks/bench_match.py __________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/benchmarks/bench_match.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
benchmarks/bench_match.py:12: in <module>
from stitches.fx_match import (
stitches/__init__.py:11: in <module>
from .fx_pangeo import fetch_nc, fetch_pangeo_table
stitches/fx_pangeo.py:8: in <module>
import intake
E ModuleNotFoundError: No module named 'intake'
_______________ ERROR collecting benchmarks/bench_processing.py ________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/benchmarks/bench_processing.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
benchmarks/bench_processing.py:11: in <module>
from stitches.fx_processing import (
stitches/__init__.py:11: in <module>
from .fx_pangeo import fetch_nc, fetch_pangeo_table
stitches/fx_pangeo.py:8: in <module>
import intake
E ModuleNotFoundError: No module named 'intake'
_________________ ERROR collecting benchmarks/bench_recipe.py __________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/benchmarks/bench_recipe.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
benchmarks/bench_recipe.py:17: in <module>
from stitches.fx_recipe import (
stitches/__init__.py:11: in <module>
from .fx_pangeo import fetch_nc, fetch_pangeo_table
stitches/fx_pangeo.py:8: in <module>
import intake
E ModuleNotFoundError: No module named 'intake'
=========================== short test summary info ============================
ERROR benchmarks/bench_match.py
ERROR benchmarks/bench_processing.py
ERROR benchmarks/bench_recipe.py
!!!!!!!!!!!!!!!!!!! Interrupted: 3 errors during collection !!!!!!!!!!!!!!!!!!!!
============================== 3 errors in 0.95s =============================== collects them when
the path is given explicitly. Verified: bare ============================= test session starts ==============================
platform darwin -- Python 3.13.3, pytest-8.3.5, pluggy-1.6.0
rootdir: /Users/d3y010/repos/github/stitches
configfile: pytest.ini
testpaths: tests
plugins: anyio-4.9.0, asyncio-1.3.0, logfire-4.21.0, hypothesis-6.152.9, langsmith-0.3.33
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
collected 0 items / 11 errors
==================================== ERRORS ====================================
__________ ERROR collecting tests/regression/test_invariance_match.py __________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/regression/test_invariance_match.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/regression/test_invariance_match.py:11: in <module>
from stitches.fx_match import (
E ModuleNotFoundError: No module named 'stitches'
_______ ERROR collecting tests/regression/test_invariance_processing.py ________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/regression/test_invariance_processing.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/regression/test_invariance_processing.py:18: in <module>
from stitches.fx_processing import (
E ModuleNotFoundError: No module named 'stitches'
_________ ERROR collecting tests/regression/test_invariance_recipe.py __________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/regression/test_invariance_recipe.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/regression/test_invariance_recipe.py:18: in <module>
from stitches.fx_match import match_neighborhood
E ModuleNotFoundError: No module named 'stitches'
__________ ERROR collecting tests/regression/test_invariance_util.py ___________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/regression/test_invariance_util.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/regression/test_invariance_util.py:16: in <module>
from stitches.fx_data import get_lat_name, global_mean
E ModuleNotFoundError: No module named 'stitches'
___________________ ERROR collecting tests/test_fx_recipe.py ___________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_fx_recipe.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_fx_recipe.py:6: in <module>
from stitches.fx_match import match_neighborhood
E ModuleNotFoundError: No module named 'stitches'
_________________ ERROR collecting tests/test_install_data.py __________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_install_data.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_install_data.py:3: in <module>
import stitches
E ModuleNotFoundError: No module named 'stitches'
_____________________ ERROR collecting tests/test_match.py _____________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_match.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_match.py:6: in <module>
from stitches.fx_match import (
E ModuleNotFoundError: No module named 'stitches'
____________________ ERROR collecting tests/test_pangeo.py _____________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_pangeo.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_pangeo.py:14: in <module>
from stitches.fx_pangeo import fetch_nc, fetch_pangeo_table
E ModuleNotFoundError: No module named 'stitches'
_____________________ ERROR collecting tests/test_seed.py ______________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_seed.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_seed.py:21: in <module>
from stitches.fx_match import match_neighborhood, shuffle_function
E ModuleNotFoundError: No module named 'stitches'
____________________ ERROR collecting tests/test_stitch.py _____________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_stitch.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_stitch.py:22: in <module>
from stitches.fx_pangeo import fetch_nc
E ModuleNotFoundError: No module named 'stitches'
_____________________ ERROR collecting tests/test_util.py ______________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/tests/test_util.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_util.py:7: in <module>
from stitches.fx_util import (
E ModuleNotFoundError: No module named 'stitches'
=========================== short test summary info ============================
ERROR tests/regression/test_invariance_match.py
ERROR tests/regression/test_invariance_processing.py
ERROR tests/regression/test_invariance_recipe.py
ERROR tests/regression/test_invariance_util.py
ERROR tests/test_fx_recipe.py
ERROR tests/test_install_data.py
ERROR tests/test_match.py
ERROR tests/test_pangeo.py
ERROR tests/test_seed.py
ERROR tests/test_stitch.py
ERROR tests/test_util.py
!!!!!!!!!!!!!!!!!!! Interrupted: 11 errors during collection !!!!!!!!!!!!!!!!!!!
============================== 11 errors in 0.71s ============================== collects 98 tests and no
benchmarks; ============================= test session starts ==============================
platform darwin -- Python 3.13.3, pytest-8.3.5, pluggy-1.6.0
rootdir: /Users/d3y010/repos/github/stitches
configfile: pytest.ini
plugins: anyio-4.9.0, asyncio-1.3.0, logfire-4.21.0, hypothesis-6.152.9, langsmith-0.3.33
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
collected 0 items / 3 errors
==================================== ERRORS ====================================
__________________ ERROR collecting benchmarks/bench_match.py __________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/benchmarks/bench_match.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
benchmarks/bench_match.py:12: in <module>
from stitches.fx_match import (
stitches/__init__.py:11: in <module>
from .fx_pangeo import fetch_nc, fetch_pangeo_table
stitches/fx_pangeo.py:8: in <module>
import intake
E ModuleNotFoundError: No module named 'intake'
_______________ ERROR collecting benchmarks/bench_processing.py ________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/benchmarks/bench_processing.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
benchmarks/bench_processing.py:11: in <module>
from stitches.fx_processing import (
stitches/__init__.py:11: in <module>
from .fx_pangeo import fetch_nc, fetch_pangeo_table
stitches/fx_pangeo.py:8: in <module>
import intake
E ModuleNotFoundError: No module named 'intake'
_________________ ERROR collecting benchmarks/bench_recipe.py __________________
ImportError while importing test module '/Users/d3y010/repos/github/stitches/benchmarks/bench_recipe.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../.pyenv/versions/3.13.3/lib/python3.13/importlib/__init__.py:88: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
benchmarks/bench_recipe.py:17: in <module>
from stitches.fx_recipe import (
stitches/__init__.py:11: in <module>
from .fx_pangeo import fetch_nc, fetch_pangeo_table
stitches/fx_pangeo.py:8: in <module>
import intake
E ModuleNotFoundError: No module named 'intake'
=========================== short test summary info ============================
ERROR benchmarks/bench_match.py
ERROR benchmarks/bench_processing.py
ERROR benchmarks/bench_recipe.py
!!!!!!!!!!!!!!!!!!! Interrupted: 3 errors during collection !!!!!!!!!!!!!!!!!!!!
============================== 3 errors in 0.29s =============================== collects exactly 33.
All benchmark calls into randomized code pass an explicit seed so the amount of
work performed is identical run to run; otherwise retry counts inside the
recipe-permutation while loop would make the timings uncomparable.
Packaging (replaces setup.py and the near-empty setup.cfg): - PEP 621 pyproject.toml with a setuptools backend. - Declares `requests`, which stitches/install_pkgdata.py imports but which was never listed anywhere. A clean install could therefore fail at install_package_data() whenever requests was not pulled in transitively. - Version single-sourced from stitches/_version.py via dynamic=["version"], replacing the build-time regex scrape of that file. - Extras split into test / docs / dev, with pytest-cov, pytest-benchmark, and pyarrow now declared rather than pip-installed ad hoc inside CI steps. pyarrow is required because golden regression artifacts are Parquet. - Drops Python 3.9 (end of life); floor is 3.10, with 3.10-3.13 classifiers. - MANIFEST.in now also ships the tests and golden artifacts so an installed distribution can be verified against recorded outputs, and prunes notebooks, docs, paper, and plans from the sdist. CI: - checkout@v3 -> v4 and setup-python@v4 -> v5. - Matrix widened from 3.9-3.11 to 3.10-3.13, with fail-fast: false so every platform failure is reported instead of only the first. - pip caching, blobless checkout (this repo's history is ~140x its working tree), and concurrency cancellation for superseded runs. - Coverage is now actually uploaded; previously coverage.xml was generated and then thrown away. - New `regression` job running the invariance suite, including a guard that fails the build if any golden artifact was modified during the run. Without that guard an accidental --update-golden would let an output change pass silently, which defeats the point of the suite. - New `benchmark` job, informational only and uploading its JSON as an artifact: shared runners are far too noisy to gate a merge on timings. - New scheduled `integration` job (network + package data, with the Zenodo download cached and keyed on record 8367628) and `latest-deps` job (unpinned resolve, continue-on-error) so upstream breakage surfaces on a schedule rather than in a user's install. This is exactly how the NumPy 2 and pandas 3 crashes fixed earlier in this branch reached a release unnoticed. Verified: package metadata reports requires-python >=3.10 and requests as a runtime dependency, example CSVs remain resolvable through importlib.resources, the workflow YAML parses, and the suite still reports 98 passed / 13 skipped.
make_tas_archive could not write its output files at all. Two defects in the
same three lines:
1. The path was built as `tas_data_dir + "/" + name + "_tas.csv"`, but
importlib.resources.files() returns a Traversable, and Traversable + str
raises TypeError.
2. Grouping used groupby(["model"]). With a single-element LIST, pandas 2.0+
yields a one-element tuple as the group name, so even after fixing the path
the filenames would have been ('BCC-CSM2-MR',)_tas.csv.
Both were verified against the real expressions before fixing. The loop is now
write_tas_data_by_model(), extracted so it can be tested without the multi-hour
Pangeo download that made this code unreachable in CI -- which is precisely why
the bug shipped. It uses os.path.join (also fixing the hardcoded "/" on Windows),
groupby("model"), and os.makedirs(exist_ok=True).
Audited every other groupby([...]) call site for the same single-key tuple issue.
Only fx_recipe.py:532 also passes a one-element list, and it discards the group
name, so it is harmless; left alone rather than churn it.
install_pkgdata was rewritten for robustness. It previously:
- buffered the entire multi-hundred-megabyte archive in memory via BytesIO;
- passed no timeout, so a hung server blocked indefinitely;
- never called raise_for_status, so a 404 HTML body was written to disk and only
failed later with a confusing "not a zip file" error;
- imported tqdm but never used it, reporting no progress on a long download;
- used os.mkdir, which fails when an intermediate parent is missing;
- silently substituted a fallback URL for unregistered versions, so a release
without a registered dataset would quietly fetch a mismatched archive;
- created temp-data but only re-nested paths containing tas-data, so temp-data
members were flattened into the top level and the directory stayed empty;
- returned None, giving callers no way to confirm what was written.
Now it streams to a temporary file with a tqdm progress bar, sets connect/read
timeouts, raises on HTTP errors, creates directories with makedirs(exist_ok=True),
warns loudly on an unregistered version, re-nests both known subdirectories,
raises an actionable RuntimeError on a corrupt archive, and returns the file
list. Logging replaces bare print calls.
While testing, extract() was found to depend on the caller having pre-created the
subdirectories, failing with an opaque FileNotFoundError from inside shutil
otherwise; it now creates its own destination layout.
Tests: 29 new (11 for make_tas_archive, 18 for the installer), all offline. The
installer tests build a local zip shaped like the real Zenodo archive and stub
the HTTP layer, so download semantics, filtering, subdirectory nesting, timeout
and error handling are all covered without network access.
Suite is now 126 passed / 13 skipped, with golden artifacts unchanged.
Preventive half of the clone-size fix. History is 288 MB against a ~2 MB working tree, and committed notebook outputs are the largest recurring contributor: notebooks/stitches-quickstart.ipynb alone accumulated 38.5 MB across 31 revisions. Base64-encoded PNGs sit on single enormous JSON lines that git cannot delta-compress, and re-running a notebook rewrites every image byte-for-byte, so each commit stores a full new copy. Measured: the four notebooks in the working tree total 7010 KB and would be 122 KB with outputs stripped, a 98% reduction. That 7 MB is what gets re-stored on every notebook commit today. Changes: - nbstripout hook, so outputs never enter history again. - check-added-large-files (--maxkb=512). The committed golden regression artifacts are unaffected; the largest is 16 KB. - check-yaml, check-toml, and check-merge-conflict, the last of which matters now that pyproject.toml is the build configuration. - pyupgrade/black/nbqa targets moved from py39 to py310 to match the new floor. Accepted trade-off: stripped notebooks show no figures in GitHub's static renderer. The executed versions are published through the nbsphinx docs build. Also corrected two overstated recommendations in the clone-performance plan after checking them against the tree: - docs/source/getting-started/output_*.png (1.65 MB) cannot simply be deleted; they are referenced by six .. image:: directives in quickstarter.rst. Removing them requires converting that page to an nbsphinx-executed notebook, so it is now tracked as a docs-restructuring task instead of a quick deletion. - stitches_diagram.jpg is confirmed byte-identical in all three locations, but the paper/ copy backs the JOSS submission and builds independently via draft-pdf.yml. Recommendation narrowed to deduplicating only the docs/ and notebooks/ copies; ~400 KB is not worth risking the published paper build. This commit deliberately does not strip the existing notebooks. Doing so is a large content change that alters what readers see on GitHub, so it belongs in its own reviewed commit; the hook prevents any further growth in the meantime.
Marker fix (the important part): `pytest -m regression` was deselecting all 70 regression tests. The marker was being attached from an autouse fixture, and later from this package's own pytest_collection_modifyitems, but `-m` filtering is evaluated by pytest's own collection hook and does not reliably observe markers added from another plugin's hook. This mattered because the CI regression job selects with `-m regression`, so the entire output-invariance suite would have reported green while running nothing -- the same class of silent-pass failure this branch removed from test_stitch.py and test_pangeo.py. Replaced with a module-level `pytestmark` in each regression module, where it cannot be missed, and left a comment in conftest.py explaining why the hook approach was abandoned so nobody reintroduces it. Verified all three selections now behave: pytest tests/regression -m regression -> 70 passed pytest tests -m "not regression" -> 56 passed, 70 deselected pytest tests -> 126 passed, 13 skipped CONTRIBUTING.md gains sections on the development environment (blobless clone, editable install with the dev extra, pre-commit), the tiered test suite and its capability flags, and the benchmark workflow. The output-invariance rules are stated explicitly: assume a regression failure is your own bug, regenerate goldens only for a genuine defect, and any golden change requires a CHANGELOG entry naming the defect plus domain-maintainer review. README.md updates the Python floor from 3.9 to 3.10, states the tested matrix, documents the blobless clone for contributors, summarizes the test commands, and links the three planning documents.
Adds a progress section to the development plan marking WS-1 through WS-4 as delivered and WS-7/WS-8 as partial, with a table of the six defects fixed. Worth recording explicitly: three of those defects were not in the original assessment. The NumPy 2 crash in get_chunk_info, the pandas 3 crash in calculate_rolling_mean, and the pytest -m regression marker bug were all found only after the regression suite and CI wiring existed. That is the concrete justification for the plan's sequencing rule that the safety net lands first. Also documents why four items were deliberately left undone rather than half-finished: hot-path optimization now has benchmarks and goldens to make it verifiable but should be its own change; the history rewrite is worth roughly an order of magnitude but rewrites every SHA and needs maintainer sign-off plus care around the Zenodo/JOSS citation trail; the committed docs renders are still referenced by quickstarter.rst; and stripping 7 MB from existing notebooks changes what readers see on GitHub and belongs in a separate reviewed commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant improvements to the project's development workflow, testing infrastructure, documentation, and packaging, with a strong emphasis on reproducibility, maintainability, and scientific rigor. The changes modernize CI, enforce output invariance, clarify contribution guidelines, and enhance pre-commit and packaging setups.
Continuous Integration and Testing Modernization:
Output Invariance and Scientific Reproducibility:
CHANGELOG.mdwith an explicit output-change policy: any change affecting scientific output must be documented, accompanied by regenerated golden artifacts, and reviewed by a domain maintainer. (CHANGELOG.md)CONTRIBUTING.mdto emphasize the importance of output invariance, explain the regression suite, and require changelog entries and invariance tests for data-producing code. (CONTRIBUTING.md) [1] [2]Pre-commit and Code Quality:
.pre-commit-config.yamlwith additional hooks for YAML/TOML checking, large file prevention, and notebook output stripping; updates Python version targets to 3.10+ to match the new minimum. [1] [2] [3] [4]Packaging and Distribution:
MANIFEST.into include the changelog, citation, requirements, test and benchmark files, and golden regression artifacts in source distributions, while excluding large notebook outputs and documentation from the package. (MANIFEST.in)Summary of Most Important Changes:
1. Testing & CI Modernization
2. Output Invariance & Documentation
CHANGELOG.mdwith a strict policy for documenting and reviewing any scientific output changes. (CHANGELOG.md)CONTRIBUTING.mdto require invariance tests and changelog entries for output changes, and details the regression and benchmark process. (CONTRIBUTING.md) [1] [2]3. Pre-commit & Code Quality
4. Packaging Improvements
MANIFEST.into include all necessary files for reproducibility and verification, including tests, benchmarks, and golden artifacts, while excluding large and unnecessary files. (MANIFEST.in)These changes collectively ensure that the project is scientifically robust, reproducible, and easier to maintain and contribute to.