Skip to content

Rename download_xl → download_xlsx, drop dead dict keys#84

Merged
nick-gorman merged 1 commit into
masterfrom
downloader-xlsx-rename
May 25, 2026
Merged

Rename download_xl → download_xlsx, drop dead dict keys#84
nick-gorman merged 1 commit into
masterfrom
downloader-xlsx-rename

Conversation

@nick-gorman
Copy link
Copy Markdown
Member

Summary

Completes the xls→xlsx work begun by #60. The filename + URL pieces (.xls.xlsx) landed in master via PR #72's ebf27ea. This PR cleans up the three remaining items that PR #72 didn't address:

  1. Rename downloader.download_xldownload_xlsx. The function downloads AEMO's NEM Registration and Exemption List, which is now a .xlsx file. The old "xl" name was ambiguous; "xlsx" matches what's on the wire.

    PR Skip/Speed up tests, resolve most open issues #67's e5cac94 (bundled inside the Session refactor) renamed it to download_xml — but that's wrong, the file isn't XML. This PR takes Matt's intended rename and uses the correct name instead.

  2. Fix the docstring, which claimed:

    "This function downloads a zipped csv using a url, extracts the csv and saves it a specified location"

    The function body is just requests.get + f.write(r.content) — no zip, no CSV, no extraction. Replaced with what it actually does.

  3. Remove two dead entries keyed by the literal string "_downloader.download_xl":

    • defaults.static_table_url["_downloader.download_xl"]
    • data_fetch_methods.static_downloader_map["_downloader.download_xl"]

    Both dicts are looked up by table name (e.g. "Generators and Scheduled Loads", "VARIABLES_FCAS_4_SECOND") — nothing ever queries them with a Python identifier as the key. git grep confirms no consumers. PR Port test suite to offline fixture-based architecture #72's xlsx fix just updated the URL value of the dead defaults entry to keep it consistent; this drops both entries entirely.

Authorship note

Written from scratch by me rather than cherry-picked, because PR #67's rename was bundled inside the Session refactor (e5cac94, a 387-line downloader rewrite) and the rename target name (download_xml) was wrong. The intent is Matt's; the corrected implementation is mine.

Test plan

  • uv run pytest tests/end_to_end_table_tests/test_static_tables.py — all 8 pass (these exercise the renamed function path via the offline mock server)
  • uv run pytest tests/ — 371 pass, 1 skipped, 1 pre-existing unrelated warning
  • CI green

Related: #60 (closed by PR #72's ebf27ea; this completes the surrounding cleanup).

Three small related cleanups around the AEMO registration list
download, completing the xls→xlsx work begun by #60 (defaults filename
+ URL pieces already done by PR #72's ebf27ea):

1. Rename `download_xl` → `download_xlsx` in downloader.py. The
   function downloads the NEM Registration and Exemption List, which
   AEMO ships as a .xlsx file (since the #60 fix). The old "xl" name
   was ambiguous; "xlsx" matches what's actually on the wire.

   PR #67's e5cac94 (bundled inside the Session refactor) renamed it
   to `download_xml` — that's wrong, the file isn't XML. This PR
   takes the rename Matt intended but uses the correct name.

2. Fix the docstring, which claimed the function "downloads a zipped
   csv using a url, extracts the csv and saves it a specified
   location". The function body is just `requests.get` + `open.write`
   — no zip, no CSV, no extraction. Replaced with what it actually
   does.

3. Remove two dead entries keyed by the literal string
   "_downloader.download_xl":

   - `defaults.static_table_url["_downloader.download_xl"]`
   - `data_fetch_methods.static_downloader_map["_downloader.download_xl"]`

   Both dicts are looked up by table-name (e.g. "Generators and
   Scheduled Loads", "VARIABLES_FCAS_4_SECOND") — nothing ever queries
   them with a Python identifier as the key. `git grep` confirms no
   consumers. PR #72's xlsx fix just updated the URL value of the
   defaults entry; this drops the entry entirely.

Existing static-table tests in tests/end_to_end_table_tests/test_static_tables.py
exercise the renamed function path via the offline mock server — all 8
pass after the rename.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nick-gorman nick-gorman merged commit af382d3 into master May 25, 2026
16 checks passed
nick-gorman added a commit that referenced this pull request May 25, 2026
PR #84 renamed download_xl to download_xlsx but this test was missed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant