ENH: cache downloaded atmosphere netCDF datasets (#654) - #1137
ENH: cache downloaded atmosphere netCDF datasets (#654)#1137thatrandomasiandev wants to merge 5 commits into
Conversation
Collapse the cache-key assignment the formatter wants on one line. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Not ready to merge yet. Three items, the third being the one I care most about. 1. Three of this PR's own new tests fail on all six platforms. 2. pylint fails (exit 8): Please split those into smaller functions rather than adding 3. Cache invalidation — the design point I want resolved before this lands. The cache is enabled by default, and the key contains neither a TTL nor the forecast model run/cycle. So a user who runs a GFS forecast today and again tomorrow silently gets yesterday's forecast, with no indication anything is stale. For forecast products that is a wrong-answer bug, not a stale-file annoyance, and it is the kind of thing that is very hard for a user to notice. What I would want:
Happy to revisit once those are addressed. |
The cache restored only the eight profile Functions, elevation and _max_expected_height, so every attribute Environment derives from the dataset was missing on the second run of an otherwise identical script. atmospheric_model_type was still set to "Forecast", so info() and all_info() walked into the Forecast branch of the prints and raised AttributeError on atmospheric_model_init_date, and to_dict() serialized those fields as None. Persist the full metadata (date range, grid bounds, and the raw interpolation inputs) alongside the profiles and reinstate it on a hit, bumping the cache format to v2. Saving also indexed temperature and both wind profiles as 2-D arrays while guarding only pressure, which raised IndexError for a constant wind and broke three existing tests. Guarding pressure alone is not sufficient either: np.asarray(None, dtype=float) yields nan, so a missing column would be written out as an entry full of NaN winds. Check every column against the pressure grid before writing. Also: - Expire forecast entries after ROCKETPY_CACHE_TTL seconds (default 6h, the GFS cycle). A launch date days out would otherwise pin the first forecast ever downloaded for it and silently reuse it forever. Reanalysis is immutable and never expires; a TTL of 0 disables expiry. - Let ROCKETPY_CACHE=0/off/false/no/none/disabled turn the cache off. It previously only relocated the directory, so there was no way to opt out globally, and an empty value cached into the working directory. - Include the variable dictionary and pressure conversion factor in the cache key, so the same source decoded two ways no longer collides. - Add clear_atmosphere_cache() to remove the entries. - Treat a damaged cache file as a miss rather than letting netCDF4's RuntimeError escape and take down the simulation with it. - Pass usedforsecurity=False to hashlib.md5 so the key still builds under a FIPS-enabled Python. - Isolate ROCKETPY_CACHE per test via an autouse fixture. The suite was writing to the real ~/.rocketpy_cache, which let one test read profiles cached by an earlier one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lint job installs pylint unpinned and has not run green on develop since 2026-07-19. Pylint 4.x reports these four disables as useless-suppression, which fails the job with exit code 8 on every open PR regardless of what the PR changes -- two of them already appear in this PR's first CI run from 2026-08-15. These files are untouched by the caching work; this commit only removes the stale comments so the lint job can go green. It is separate so it can be dropped and landed on its own if maintainers prefer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1137 +/- ##
===========================================
- Coverage 91.51% 91.37% -0.15%
===========================================
Files 131 132 +1
Lines 17733 18131 +398
===========================================
+ Hits 16229 16567 +338
- Misses 1504 1564 +60 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
.ncfiles under~/.rocketpy_cache/atmosphere(override withROCKETPY_CACHE).set_atmospheric_modelcalls reuse disk.no_cache=FalsetoEnvironment.set_atmospheric_model(and Windy processing) so callers can force a fresh download without breaking existing callers.Notes / TODOs
OPeNDAP "Best" aggregations are virtual catalogs, not downloadable files, so this PR caches the extracted profiles RocketPy already materializes after the first fetch (same end-user outcome: reruns skip the network). Remaining follow-ups:
.nc(non-OPeNDAP)Fixes #654
Test plan
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest tests/unit/environment/test_atmosphere_cache.pyno_cache=Trueforces re-download