Auto-create raw_data_location in dynamic_data_compiler and static_table#101
Merged
Merged
Conversation
`cache_compiler` already created the cache directory if it didn't yet
exist; the other two entry points raised `UserInputError`. Same parameter
on three sibling APIs behaving differently is API friction: every new
user has to run a separate `mkdir` for their first `dynamic_data_compiler`
call, and the inconsistency is unexplained.
Lift `cache_compiler`'s pattern (including the nicer "exists as a file,
not a directory" error message it already had) into a shared
`_validate_raw_data_location` helper called from all three entry points.
Behaviour matrix now consistent across `dynamic_data_compiler`,
`cache_compiler`, and `static_table`:
None -> UserInputError("...is None")
path is a file -> UserInputError("...exists as a file, not a directory")
path doesn't exist -> _os.makedirs(path)
path exists as dir -> no-op
Tests:
- Drop the two `*_missing_includes_path_in_error` tests on dynamic/static
(they asserted the now-obsolete raise-on-missing behaviour).
- Add `*_is_a_file` and `*_missing_is_auto_created` for all three entry
points (6 new tests; cache_compiler now has explicit coverage of the
file-vs-dir distinction it already implemented).
Closes the "raw_data_cache is auto-created if missing" release-notes
bullet, which was previously only true for `cache_compiler`.
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
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.
Summary
cache_compileralready creates the cache directory if it doesn't yet exist; the other two entry points raisedUserInputError. Same parameter on three sibling APIs behaving differently is real API friction — every new user has tomkdirseparately before their firstdynamic_data_compilercall, and the inconsistency is unexplained.This PR lifts
cache_compiler's pattern (including the better "exists as a file, not a directory" error wording it already had) into a shared_validate_raw_data_locationhelper called from all three entry points.Behaviour now consistent across
dynamic_data_compiler,cache_compiler, andstatic_table:raw_data_locationvalueNoneUserInputError("...is None")UserInputError("...exists as a file, not a directory")os.makedirs(path)Why
mkdirstep before their first NEMOSIS call.dynamic_data_compilerandstatic_tablepreviously gave a worse error when the path was a file (collapsed into "does not exist"); the shared helper fixes that.cache_compiler.Trade-off considered: typo silence
Auto-create means a typo like
nem_cahcesilently creates a new empty dir at the typo location rather than raising — the user wonders why their first download is slow. The countervailing cost is forcing every user tomkdironce. Typos are rare and trivially recoverable (re-run with the correct path); the mkdir tax hits every new install. The trade-off favours auto-create, and that's already the established behaviour incache_compiler.Tests
*_missing_includes_path_in_errortests on dynamic/static (asserted the now-obsolete raise-on-missing behaviour).*_is_a_fileand*_missing_is_auto_createdfor all three entry points (6 new tests;cache_compilernow has explicit coverage of the file-vs-dir distinction it already implemented).Test plan
uv run pytest tests/— 439 passed, 1 skipped (was 433 + 1 on master pre-change; net +6 from the new tests).🤖 Generated with Claude Code