From 45285d266e58222d0cff2f560e860fafa40b003f Mon Sep 17 00:00:00 2001 From: nick-gorman Date: Tue, 26 May 2026 08:53:38 +1000 Subject: [PATCH] Flip `keep_zip` default to `True` for `dynamic_data_compiler` and `cache_compiler` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #87 had set `keep_zip=False` by default for a leaner cache, paired with `keep_csv=False`. In practice the slow-internet / re-download use case from #56 is the more common need — having the compressed archive on disk means subsequent runs (cache rebuilds, format changes, re-runs on a flaky connection) don't have to re-fetch from AEMO. Matt's original streaming work in `2a5812f` left zips on disk after extraction for this reason; this change brings the default back in line with that intent. `keep_csv=False` stays as-is (raw CSVs are cheap to re-extract from the retained zip, so there's no symmetric pressure to keep them). Changes: 1. `dynamic_data_compiler`, `cache_compiler`, `_dynamic_data_fetch_loop`, `_download_data` (data_fetch_methods.py) — `keep_zip` default flips to `True`. Docstrings updated: "True by default — keeps the compressed archive so subsequent runs don't have to re-download from AEMO (see #56). Set False for a leaner cache when re-download cost is not a concern." 2. All internal helpers in `downloader.py` get the same default flip so any internal caller that hits the default sees the same value as the public API: `run`, `run_bid_tables`, `run_next_day_region_tables`, `run_next_dispatch_tables`, `run_intermittent_gen_scada`, `run_fcas4s`, the four `_download_and_unpack_*` helpers, and `download_unzip_csv`. `download_unzip_csv`'s docstring swaps which branch is annotated `(default)`. 3. Tests in `tests/end_to_end_table_tests/test_cache_compiler.py`: - `test_keep_zip_default_is_false` renamed to `test_keep_zip_default_is_true` with inverted assertion. - `test_keep_zip_true_retains_zip` keeps its explicit `keep_zip=True` to lock the contract independent of the default; docstring notes that it's now also the default. - New `test_keep_zip_false_removes_zip` covers the explicit opt-out path. The dangling reference to this test in `test_fformat_csv.py` pre-dated this PR; that reference is now fixed to point at the real location. The `(path, downloaded)` cleanup contract from PR #87 is preserved — when a caller does pass `keep_zip=False`, cleanup only fires for zips this call actually wrote, so concurrent-same-cache use is still safe. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/nemosis/data_fetch_methods.py | 25 +++++++----- src/nemosis/downloader.py | 39 ++++++++++--------- .../test_cache_compiler.py | 33 ++++++++++++---- .../test_fformat_csv.py | 3 +- 4 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/nemosis/data_fetch_methods.py b/src/nemosis/data_fetch_methods.py index fdc7ad0..34da3ed 100644 --- a/src/nemosis/data_fetch_methods.py +++ b/src/nemosis/data_fetch_methods.py @@ -25,7 +25,7 @@ def dynamic_data_compiler( filter_values=None, fformat="feather", keep_csv=False, - keep_zip=False, + keep_zip=True, parse_data_types=True, rebuild=False, **kwargs, @@ -61,9 +61,11 @@ def dynamic_data_compiler( is written. False by default — lean cache. keep_zip (bool): If True, downloaded AEMO archive zips are retained in the cache directory after extraction. - False by default. Set True on slow connections - to avoid re-downloading on subsequent runs - (see #56). + True by default — keeps the compressed archive so + subsequent runs (e.g. cache rebuilds, format + changes) don't have to re-download from AEMO + (see #56). Set False for a leaner cache when + re-download cost is not a concern. data_merge (bool): concatenate DataFrames and return one DataFrame. If False, will not return any data. parse_data_types (bool): infers data types of columns when reading @@ -174,7 +176,7 @@ def cache_compiler( fformat="feather", rebuild=False, keep_csv=False, - keep_zip=False, + keep_zip=True, **kwargs, ): """ @@ -211,9 +213,12 @@ def cache_compiler( cache is built. False by default — lean cache. keep_zip (bool): If True, downloaded AEMO archive zips are retained in the cache directory after - extraction. False by default. Set True on - slow connections to avoid re-downloading on - subsequent runs (see #56). + extraction. True by default — keeps the + compressed archive so subsequent runs (e.g. + cache rebuilds, format changes) don't have to + re-download from AEMO (see #56). Set False + for a leaner cache when re-download cost is + not a concern. **kwargs: additional arguments passed to the pd.to_{fformat}() function Returns: @@ -593,7 +598,7 @@ def _dynamic_data_fetch_loop( date_filter, fformat="feather", keep_csv=False, - keep_zip=False, + keep_zip=True, caching_mode=False, rebuild=False, write_kwargs={}, @@ -846,7 +851,7 @@ def _write_to_format(data, fformat, full_filename, write_kwargs): def _download_data( table_name, table_type, filename_stub, day, month, year, chunk, index, raw_data_location, - keep_zip=False, + keep_zip=True, ): """ Dispatch table to downloader to be downloaded. diff --git a/src/nemosis/downloader.py b/src/nemosis/downloader.py index aaa86a5..55fc643 100644 --- a/src/nemosis/downloader.py +++ b/src/nemosis/downloader.py @@ -100,7 +100,7 @@ def _pre_check_file_is_missing(file_url): return True -def run(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=False): +def run(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=True): """This function""" url = defaults.aemo_mms_url @@ -115,7 +115,7 @@ def run(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=Fa logger.warning(f"{filename_stub} not downloaded ({e})") -def run_bid_tables(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=False): +def run_bid_tables(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=True): if day is None: run(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=keep_zip) else: @@ -131,7 +131,7 @@ def run_bid_tables(year, month, day, chunk, index, filename_stub, down_load_to, logger.warning(f"{filename_stub} not downloaded ({e})") -def run_next_day_region_tables(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=False): +def run_next_day_region_tables(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=True): try: filename_stub = "PUBLIC_DAILY_{year}{month}{day}".format(year=year, month=month, day=day) download_url = _get_current_url( @@ -144,7 +144,7 @@ def run_next_day_region_tables(year, month, day, chunk, index, filename_stub, do logger.warning(f"{filename_stub} not downloaded ({e})") -def run_next_dispatch_tables(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=False): +def run_next_dispatch_tables(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=True): try: filename_stub = "PUBLIC_NEXT_DAY_DISPATCH_{year}{month}{day}".format(year=year, month=month, day=day) download_url = _get_current_url( @@ -155,7 +155,7 @@ def run_next_dispatch_tables(year, month, day, chunk, index, filename_stub, down logger.warning(f"{filename_stub} not downloaded ({e})") -def run_intermittent_gen_scada(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=False): +def run_intermittent_gen_scada(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=True): try: download_url = _get_current_url( filename_stub, @@ -173,7 +173,7 @@ def _get_current_url(filename_stub, current_page_url): def _download_and_unpack_bid_move_complete_files( - download_url, down_load_to, keep_zip=False + download_url, down_load_to, keep_zip=True ): zip_local_path, downloaded = download_to_dir(download_url, down_load_to) try: @@ -213,7 +213,7 @@ def _download_and_unpack_bid_move_complete_files( def _download_and_unpack_next_region_tables( - download_url, down_load_to, keep_zip=False + download_url, down_load_to, keep_zip=True ): zip_local_path, downloaded = download_to_dir(download_url, down_load_to) try: @@ -246,7 +246,7 @@ def _download_and_unpack_next_region_tables( def _download_and_unpack_next_dispatch_load_files_complete_files( - download_url, down_load_to, keep_zip=False + download_url, down_load_to, keep_zip=True ): zip_local_path, downloaded = download_to_dir(download_url, down_load_to) try: @@ -275,7 +275,7 @@ def _download_and_unpack_next_dispatch_load_files_complete_files( def _download_and_unpack_intermittent_gen_scada_file( - download_url, down_load_to, keep_zip=False + download_url, down_load_to, keep_zip=True ): zip_local_path, downloaded = download_to_dir(download_url, down_load_to) try: @@ -321,7 +321,7 @@ def _find_start_row_nth_table(sub_folder_zipfile, file_name, n): -def run_fcas4s(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=False): +def run_fcas4s(year, month, day, chunk, index, filename_stub, down_load_to, keep_zip=True): """This function""" # Add the year and month information to the generic AEMO data url @@ -412,20 +412,21 @@ def download_to_path(url, path_and_name, force_redo=False): return True -def download_unzip_csv(url, down_load_to, keep_zip=False): +def download_unzip_csv(url, down_load_to, keep_zip=True): """ Download a zipped csv from a URL, extract its contents into `down_load_to`, and (per `keep_zip`) retain the zip on disk. - `keep_zip=False` (default) cleans up the zip after extracting, - leaving only the extracted CSV in the cache directory. Cleanup - only touches zips this call actually downloaded — pre-existing - zips (from a previous call, or another concurrent process) are - left alone. + `keep_zip=True` (default) keeps the compressed archive on disk + after extraction, so subsequent calls (e.g. cache rebuilds, + format changes, slow connections per #56) can re-extract + without re-downloading. - `keep_zip=True` addresses #56: a cached zip on disk so subsequent - calls can re-extract without re-downloading (useful on slow - connections). + `keep_zip=False` cleans up the zip after extracting, leaving + only the extracted CSV in the cache directory. Cleanup only + touches zips this call actually downloaded — pre-existing zips + (from a previous call, or another concurrent process) are left + alone. """ zip_local_path, downloaded = download_to_dir(url, down_load_to) try: diff --git a/tests/end_to_end_table_tests/test_cache_compiler.py b/tests/end_to_end_table_tests/test_cache_compiler.py index 2b7c1f7..0d5e0a2 100644 --- a/tests/end_to_end_table_tests/test_cache_compiler.py +++ b/tests/end_to_end_table_tests/test_cache_compiler.py @@ -155,10 +155,11 @@ def test_existing_feather_means_no_csv_is_fetched(nemosis_fixture): ) -def test_keep_zip_default_is_false(nemosis_fixture): - """Default behaviour — keep_zip=False removes the downloaded zip - after extracting the CSV. Lean cache; only the typed feather (and - optional CSV if keep_csv=True) remains.""" +def test_keep_zip_default_is_true(nemosis_fixture): + """Default behaviour — keep_zip=True retains the downloaded zip + after extracting the CSV, so subsequent runs (cache rebuilds, + format changes, slow connections per #56) can re-extract without + re-downloading.""" cache_compiler( start_time=START, end_time=END, table_name="DISPATCHPRICE", @@ -167,13 +168,14 @@ def test_keep_zip_default_is_false(nemosis_fixture): # no keep_zip kwarg — exercises the default ) zip_files = list(nemosis_fixture.glob("*.zip")) - assert not zip_files, f"default keep_zip=False should remove zips; found: {zip_files}" + assert zip_files, "default keep_zip=True should retain the downloaded zip" def test_keep_zip_true_retains_zip(nemosis_fixture): - """keep_zip=True is the opt-in for #56's slow-internet use case — - the AEMO archive zip stays on disk after extraction so subsequent - runs can re-extract without re-downloading.""" + """keep_zip=True (the default, per #56's slow-internet use case) + leaves the AEMO archive zip on disk after extraction so subsequent + runs can re-extract without re-downloading. Exercised here with + an explicit True to lock the contract independent of the default.""" cache_compiler( start_time=START, end_time=END, table_name="DISPATCHPRICE", @@ -185,6 +187,21 @@ def test_keep_zip_true_retains_zip(nemosis_fixture): assert zip_files, "keep_zip=True should retain the downloaded zip" +def test_keep_zip_false_removes_zip(nemosis_fixture): + """keep_zip=False is the explicit opt-out for callers who want a + lean cache (no compressed archives on disk). The downloaded zip + must be cleaned up after extracting the CSV.""" + cache_compiler( + start_time=START, end_time=END, + table_name="DISPATCHPRICE", + raw_data_location=str(nemosis_fixture), + rebuild=True, + keep_zip=False, + ) + zip_files = list(nemosis_fixture.glob("*.zip")) + assert not zip_files, f"keep_zip=False should remove zips; found: {zip_files}" + + def test_cached_zip_extracts_without_network(nemosis_fixture, monkeypatch): """The #56 benefit in action: with keep_zip=True on a first call, a subsequent call that needs the same CSV but finds the feather diff --git a/tests/end_to_end_table_tests/test_fformat_csv.py b/tests/end_to_end_table_tests/test_fformat_csv.py index 3f0825a..9aefe8f 100644 --- a/tests/end_to_end_table_tests/test_fformat_csv.py +++ b/tests/end_to_end_table_tests/test_fformat_csv.py @@ -36,7 +36,8 @@ def test_keep_csv_false_removes_csv(nemosis_fixture): use case is slow internet, so caching the compressed archive avoids re-downloading on subsequent runs). Users who want a truly empty cache can pass keep_zip=False — covered by - test_keep_zip_false_removes_zip in tests/test_downloader.py. + test_keep_zip_false_removes_zip in + tests/end_to_end_table_tests/test_cache_compiler.py. """ dynamic_data_compiler( start_time="2018/05/01 00:00:00",