diff --git a/src/nemosis/data_fetch_methods.py b/src/nemosis/data_fetch_methods.py index 79e1522..b0916e8 100644 --- a/src/nemosis/data_fetch_methods.py +++ b/src/nemosis/data_fetch_methods.py @@ -16,6 +16,26 @@ logger = logging.getLogger(__name__) +def _validate_raw_data_location(raw_data_location): + """Validate (and create-if-needed) the user's raw_data_location. + + Common to dynamic_data_compiler, cache_compiler, and static_table. + Rejects None (typo / unset config) and paths that exist as files + (clearer than the downstream 'not a directory' error). If the path + doesn't yet exist as a directory, create it — first-run UX is + smoother than forcing every caller to mkdir up front. + """ + if raw_data_location is None: + raise UserInputError("The raw_data_location provided is None.") + if _os.path.isfile(raw_data_location): + raise UserInputError( + f"The raw_data_location {raw_data_location} provided " + f"exists as a file, not a directory." + ) + if not _os.path.isdir(raw_data_location): + _os.makedirs(raw_data_location) + + def dynamic_data_compiler( start_time, end_time, @@ -80,13 +100,7 @@ def dynamic_data_compiler( all_data (pd.Dataframe): All data concatenated. """ - if raw_data_location is None: - raise UserInputError("The raw_data_location provided is None.") - - if not _os.path.isdir(raw_data_location): - raise UserInputError( - f"The raw_data_location {raw_data_location} provided does not exist." - ) + _validate_raw_data_location(raw_data_location) if table_name not in _defaults.dynamic_tables: raise UserInputError("Table name provided is not a dynamic table.") @@ -255,14 +269,7 @@ def cache_compiler( Nothing """ - if raw_data_location is None: - raise UserInputError("The raw_data_location provided is None.") - - if not _os.path.isdir(raw_data_location): - if _os.path.isfile(raw_data_location): - raise UserInputError(f"The raw_data_location {raw_data_location} provided exists as a file, not directory.") - else: - _os.makedirs(raw_data_location) + _validate_raw_data_location(raw_data_location) if table_name not in _defaults.dynamic_tables: raise UserInputError("Table name provided is not a dynamic table.") @@ -362,13 +369,7 @@ def static_table( "The 'Generators and Scheduled Loads' table still works." ) - if raw_data_location is None: - raise UserInputError("The raw_data_location provided is None.") - - if not _os.path.isdir(raw_data_location): - raise UserInputError( - f"The raw_data_location {raw_data_location} provided does not exist." - ) + _validate_raw_data_location(raw_data_location) if table_name not in _defaults.static_tables: raise UserInputError("Table name provided is not a static table.") diff --git a/tests/test_errors.py b/tests/test_errors.py index cc1bf22..22d963a 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -110,24 +110,34 @@ def test_dynamic_raw_data_location_is_none(): ) -def test_dynamic_raw_data_location_missing_includes_path_in_error(tmp_path): - """The "does not exist" error must include the offending path so the - caller can see exactly what they passed (helps diagnose typos and - cwd mistakes). Regression test for the improvement that landed via - the doc-strings-and-error-messages cleanup.""" - bogus = str(tmp_path / "subdir-that-does-not-exist") - with pytest.raises(UserInputError, match="subdir-that-does-not-exist"): +def test_dynamic_raw_data_location_is_a_file(tmp_path): + """If the caller hands us a path that already exists as a file (typo + pointing at a script, or a regular file shadowing the intended dir + name), raise a clear UserInputError naming the path and the cause + rather than the downstream "not a directory" OSError. Contract is + consistent across all three entry points.""" + file_path = tmp_path / "not-a-dir.txt" + file_path.write_text("hello") + with pytest.raises(UserInputError, match="exists as a file"): dynamic_data_compiler( "2018/05/01 00:00:00", "2018/05/01 01:00:00", - "DISPATCHPRICE", raw_data_location=bogus, + "DISPATCHPRICE", raw_data_location=str(file_path), ) -def test_static_raw_data_location_missing_includes_path_in_error(tmp_path): - """Same regression contract for static_table.""" - bogus = str(tmp_path / "another-missing-dir") - with pytest.raises(UserInputError, match="another-missing-dir"): - static_table("VARIABLES_FCAS_4_SECOND", raw_data_location=bogus) +def test_dynamic_raw_data_location_missing_is_auto_created(nemosis_fixture): + """A missing raw_data_location is auto-created so first-run callers + don't need a separate `mkdir` step. Previously dynamic_data_compiler + raised UserInputError here; now it matches cache_compiler's + longstanding behaviour.""" + new_dir = nemosis_fixture / "fresh-cache-subdir" + assert not new_dir.exists() + df = dynamic_data_compiler( + "2018/05/01 00:00:00", "2018/05/01 01:00:00", + "DISPATCHPRICE", raw_data_location=str(new_dir), + ) + assert new_dir.is_dir() + assert not df.empty # --------------------------------------------------------------------------- @@ -169,6 +179,31 @@ def test_cache_raw_data_location_is_none(): ) +def test_cache_raw_data_location_is_a_file(tmp_path): + file_path = tmp_path / "not-a-dir.txt" + file_path.write_text("hello") + with pytest.raises(UserInputError, match="exists as a file"): + cache_compiler( + "2018/05/01 00:00:00", "2018/05/01 01:00:00", + "DISPATCHPRICE", raw_data_location=str(file_path), + ) + + +def test_cache_raw_data_location_missing_is_auto_created(nemosis_fixture): + """cache_compiler has auto-created the cache dir for a while; this + test pins that behaviour so the shared validator can't accidentally + regress it.""" + new_dir = nemosis_fixture / "fresh-cache-subdir" + assert not new_dir.exists() + cache_compiler( + "2018/05/01 00:00:00", "2018/05/01 01:00:00", + "DISPATCHPRICE", raw_data_location=str(new_dir), + ) + assert new_dir.is_dir() + # And cache_compiler actually wrote something into it. + assert any(new_dir.iterdir()) + + # --------------------------------------------------------------------------- # Argument-validation: static_table # --------------------------------------------------------------------------- @@ -226,6 +261,23 @@ def test_static_raw_data_location_is_none(): static_table("VARIABLES_FCAS_4_SECOND", raw_data_location=None) +def test_static_raw_data_location_is_a_file(tmp_path): + file_path = tmp_path / "not-a-dir.txt" + file_path.write_text("hello") + with pytest.raises(UserInputError, match="exists as a file"): + static_table("VARIABLES_FCAS_4_SECOND", raw_data_location=str(file_path)) + + +def test_static_raw_data_location_missing_is_auto_created(nemosis_fixture): + """Same contract as the dynamic/cache entry points — first-run + callers shouldn't need a separate mkdir.""" + new_dir = nemosis_fixture / "fresh-cache-subdir" + assert not new_dir.exists() + df = static_table("VARIABLES_FCAS_4_SECOND", raw_data_location=str(new_dir)) + assert new_dir.is_dir() + assert not df.empty + + # --------------------------------------------------------------------------- # Network / availability: NoDataToReturn # ---------------------------------------------------------------------------