Skip to content

Reject raw_data_location=None with a clear UserInputError#80

Merged
nick-gorman merged 2 commits into
masterfrom
reject-none-raw-data-location
May 25, 2026
Merged

Reject raw_data_location=None with a clear UserInputError#80
nick-gorman merged 2 commits into
masterfrom
reject-none-raw-data-location

Conversation

@nick-gorman
Copy link
Copy Markdown
Member

Summary

Salvages the one source-side improvement from #67's test-triage bundle that survived PR #72 — explicit None check on raw_data_location in all three public entrypoints. Today, passing None falls through to os.path.isdir(None) and raises TypeError from the stdlib, with a message that doesn't mention the parameter. Now it raises UserInputError (the library's contract for caller-input mistakes) with a message that names the parameter.

Two commits:

  • Reject raw_data_location=None with a clear UserInputError (author: @mdavis-xyz) — cherry-picked from Skip/Speed up tests, resolve most open issues #67 commit 7e71e73, source change only. Adds the check to dynamic_data_compiler and cache_compiler. The companion tests/test_00_test_setup.py from the original commit is dropped: it asserted defaults.raw_data_cache is set via the NEMOSIS_TEST_CACHE env var, which Port test suite to offline fixture-based architecture #72 obsoleted by moving to fixture-scoped temp dirs via nemosis_fixture.
  • Extend None-check to static_table and add regression tests (author: me) — applies the same check to static_table (the original commit didn't, but it has the same _os.path.isdir line and the same UX flaw), and adds one regression test per entrypoint to tests/test_errors.py.

Context — what else is left from PR #67's test triage

Audited all six test commits from #67 against post-#72 master:

#67 commit Verdict
dd01b63 skip cache-purging tests (#16) Redundant — target tests/test_format_options.py deleted by #72
e3dbef5 skip FCAS_4_SECOND tests (#64) Redundant — target tests/test_data_fetch_methods.py deleted; no live-AEMO FCAS tests left
397bdf8 un-hardcode cache path Redundant — target tests/test_errors_and_warnings.py deleted; new test_errors.py uses fixture path
fea56a0 skip long processing_info_maps tests Redundant — test_processing_info_maps.py rewritten by #72, now offline + fast
7e71e73 cache-path robustness This PR — source change kept, test file dropped
238fd35 typo fix in test_00_test_setup.py Redundant — applied to the file we're dropping

Test plan

  • uv run pytest tests/test_errors.py — 16/16 pass (13 pre-existing + 3 new)
  • CI green

mdavis-xyz and others added 2 commits May 25, 2026 11:31
Cherry-picked source change from #67 (commit 7e71e73). The companion
tests/test_00_test_setup.py from the original commit is dropped — it
asserted that defaults.raw_data_cache is set, which PR #72 made obsolete
when the suite switched to fixture-scoped temp directories via
nemosis_fixture.

Without this check, passing raw_data_location=None falls through to
os.path.isdir(None) and raises TypeError from inside the stdlib, with
a message that doesn't mention the parameter. UserInputError is the
contract for caller-input mistakes, and the message now names the
parameter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply the same UserInputError("...is None") guard to static_table that
the previous commit added to dynamic_data_compiler and cache_compiler.
The original cherry-picked work (PR #67) didn't cover static_table, but
it has the same `_os.path.isdir(raw_data_location)` line and the same
TypeError-from-stdlib UX flaw on None input.

Add three regression tests to tests/test_errors.py — one per public
entrypoint — locking in the contract that None raises UserInputError
with a message naming the parameter. The tests don't take the
nemosis_fixture parameter because the None check fires before any
disk or network access.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nick-gorman nick-gorman merged commit ea10bc3 into master May 25, 2026
16 checks passed
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.

2 participants