Repository navigation
feat: unified data interface (ADR 0001) — hydrodatasource alignment - #14
Merged
Merged
Conversation
…rsion logic; optimize the selection logic of long-term data sets
…urce reader classes - Register all HydroData subclasses: floodevent, selfmade, longterm, forecast, station, tghydro, gages, grdc, rainfall, crd, rsvrinflow - Implement resolve_data_path() following same contract as hydromodel ADR 0001 - Merge hydrodataset's READER_ALIASES (37 entries) for a combined 44+ registry - Re-export DatasetResolutionError from hydrodataset - Add comprehensive tests (19 tests covering aliases, errors, local/cloud resolution) Co-Authored-By: Claude <noreply@anthropic.com>
- Add tool.uv.sources pointing to local hydrodataset for development - Include spec document for unified data interface alignment Co-Authored-By: Claude <noreply@anthropic.com>
- Register songliao_event → floodevent reader with relative path - Follows ADR 0001 format: dataset id → reader alias + relative path Co-Authored-By: Claude <noreply@anthropic.com>
- Remove postgres globals (POSTGRES_PARAM, PS, get_postgres_connection) - Remove minio clients unused externally (RO, S3, MC) - Remove unused constants (STATION_BUCKET, STATION_OBJECT, GRID_INTERIM_BUCKET) - Remove minio/postgres validation from read_setting() - Add bridge: translate new storage.* format → legacy local_data_path keys - Add pure-function APIs: get_local_root(), get_cache_dir(), get_storage_config() - Delegate to hydrodataset.configs.settings for new-format resolution Co-Authored-By: Claude <noreply@anthropic.com>
- HydroData base: accept optional uri kwarg, bypass os.path.join() - All subclasses pass uri through to super().__init__() - Fix FloodEventDatasource: remove leftover download=False kwarg - Add pathlib.Path import to data_source.py - All classes verified via inspect.signature check Co-Authored-By: Claude <noreply@anthropic.com>
…r from package Co-Authored-By: Claude <noreply@anthropic.com>
… exports - test_reader_uri_constructors.py: 27 tests covering HydroData base, Pattern A (7 subclasses via inspect.signature), Pattern B (4 subclasses via mock), and SelfMadeHydroDataset real instantiation with tmp_path - test_config_api.py: 17 tests for read_setting validation, get_local_root/get_cache_dir/get_storage_config APIs, backward-compat globals, and format bridging - test_package_exports.py: 7 tests verifying READER_ALIASES, resolve_data_path, and DatasetResolutionError are importable from hydrodatasource top-level package, with merged alias counts All 70 new tests (51 new + 19 existing test_data_resolver.py) pass GREEN.
…/return bugs
- data_resolver.py: drop private _load_registry import, use in-memory
_HDS_DATASETS + extra_registry_dicts injection. Add local_root passthrough.
Remove _load_yaml/_validate_relative_path/parent.parent.parent path chain.
Net reduction: 164 → 118 lines (-28%).
- floodevent.py: skip os.path.exists with self.head != 'minio' guard for
S3/cloud URIs. Fix return {} → return None (2 sites) to avoid empty dict
passing is-not-None gate.
- grdc.py/reservoir_datasets.py/rainfall_reader.py/rsvr_inflow_reader.py:
add self.dataset_name in Pattern B readers that bypass super().__init__().
- tests: add test_resolve_with_local_root_override, TestS3UriPathCheck,
test_pattern_b_readers_have_dataset_name (4 parametrized). Fix mock
paths for get_local_root/get_storage_config to hydrodataset namespace.
- config.py: replace import-time _init_settings() call with module __getattr__ lazy loading. SETTING/CACHE_DIR/LOCAL_DATA_PATH/FS/MINIO_PARAM are now initialized on first access, eliminating import-time I/O and S3 connection attempts. - CACHE_DIR now delegates to hydrodataset's get_cache_dir(), ensuring a single canonical cache path across repos. Removes the divergence where CACHE_DIR and get_cache_dir() returned different directories. - _init_settings() cleans stale __dict__ entries (left by monkeypatch teardowns) to keep __getattr__ resolution correct across test runs. - tests: add test_cache_dir_consistent_with_get_cache_dir
…reader classes - Replace (data_path, dataset_name) with single uri parameter in HydroData base - All 11 reader classes now accept uri (positional for Pattern B, keyword for Pattern A) - Remove legacy kwargs (data_path, dataset_name, data_folder) from all constructors - Add S3 URI guard: skip os.path.exists() when head='minio' in FloodEventDatasource - Add dataset_name auto-derivation from URI basename in HydroData.__init__ - Lazy-load config globals: rename LOCAL_DATA_PATH to LOCAL_ROOT, add warnings - Add data_resolver.py thin wrapper over hydrodataset public API with extra_registry_dicts - Add datasets.yml registry with songliao_event → floodevent mapping - Export READER_ALIASES, resolve_data_path, DatasetResolutionError from package - Add 74 tests covering URI constructors, config API, data resolver, and package exports - Align CACHE_DIR with hydrodataset defaults Closes #13
…er config Datasets registry lives in _HDS_DATASETS (in-code, Layer 2 of 3-layer cascade). configs/datasets.yml is a Layer 3 user override — the library does not ship one. Changes: - Delete configs/datasets.yml (redundant with _HDS_DATASETS) - Update data_resolver.py comments to document the 3-layer architecture - Add tests: resolution without YAML, _HDS_DATASETS entry verification All 69 relevant tests pass.
Make the dataset registry dict a public name since downstream code may need to inspect or extend it. Add __all__ to explicitly control what's exported from the data_resolver module.
…ontext API Expose a thin open_dataset factory that injects HDS registry/aliases so callers can instantiate readers in one line, and migrate resolve_data_path to the shared ctx-based contract. Co-authored-by: Cursor <cursoragent@cursor.com>
Read attributes from either attributes.csv (preferred, backward compatible) or a NetCDF file in the attributes/ dir, unifying csv/nc and local/minio via a single _read_attributes_df() helper. basins.shp becomes optional in cache_attributes_xrdataset: fall back to the 'area' column from attributes when no shape file is present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On a fresh machine the resolved CACHE_DIR (e.g. ~/.cache/hydrodataset) may not exist yet; h5netcdf/os.listdir do not create it, so caching attributes/ timeseries raised FileNotFoundError. Create the dir once at CACHE_DIR resolution with makedirs(exist_ok=True). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Data source hardening: - require uri in HydroData, reject legacy data_path/dataset_name/data_folder kwargs - preserve zero-padded basin_id via dtype passthrough in access_fs - fix streamflow_unit mapping '1MS'->'1M'; add GLOBAL_DIR to LongTermDataset - floodevent: filter on 'basin' coord (read side), not gage_id - data_resolver: source=None falls back to storage.default_source - config: warn when hydro_setting.yml lacks a 'storage' section - remove dead scripts/example_tg_usage.py hydroutils 0.2.0 migration (hydro_log module removed): - replace @hydro_logger decorator with stdlib logging in gages, rainfall_reader, rainfall_cleaner, rsvr_inflow_cleaner; fixes latent .info() call on the decorator function in gages.prepare_usgs_data - bump dependency to hydroutils>=0.2.0 Tests: - add regression tests (test_fix_*, test_hydroutils_compat) - fix CACHE_DIR isolation and sys.modules teardown pollution - de-hardcode package export count
Cleaner base: - initialize origin_df/processed_df; read_data loads CSV with utf-8/gbk fallback - fix chained-assignment warning in streamflow anomaly_process via .loc - waterlevel: fix if/elif so moving_grad no longer warns; remove logger_level dead param Error handling: - streamflow_cleaner: drop try/except that masked KeyError as NameError (remove WTF print) - utils: minio_file_list propagates FS errors (graceful [] when S3 unconfigured) - utils: is_minio_folder normalizes s3:// prefix, handles empty dirs, propagates real errors Data source: - HydroData rejects empty uri; streamflow_unit adds 8D mapping and tolerates time_unit=None - remove 3 dead cargo-cult import pint_xarray lines (fixed full-suite pint flaky test) - dmca_esr: remove debug prints; scripts: migrate station_dataset_usage to uri= grdc: - replace assert with explicit ValueError (safe under -O); numeric (not lexicographic) id ordering - remove __main__ block that triggered full cache build; drop orphan get_local_root import Dead code (net -616 lines): - remove 11 unused utils unit-conversion helpers + 9 orphaned imports - delete unreferenced reader/minio_api.py module - remove orphan _rsvr_rolling_window_abrupt_abnormal_rm and deprecated voronoi functions - strip commented-out debug lines and garbage log call in rainfall_cleaner Tests: - add 8 regression test files (cleaner, error handling, nits, pint imports, unit mapping, dead code, suggestions, grdc ordering) - fix data_source one_day_dataset fixture and relax brittle compat assertion Deps: uv.lock synced for all extras (dev tooling)
…>=0.3.0 README (en/zh): - describe the unified URI-only design and hydrodataset compatibility (drop internal ADR framing) - add open_dataset() quick start; URI-only SelfMadeHydroDataset construction - 11 reader alias table; storage.* config; directory structure with intermediate/stations/8D docs/: - reader: open_dataset/resolve_data_path as the main entry, URI-only examples, full alias table - configs: storage.* unified format + default_source + resolution flow - api: add data_resolver section and all reader classes - cleaner: note the functional Cleaner base and correct algorithm names - remove spec-unified-data-interface.md (design now lives in data_resolver.py docstring) pyproject: hydrodataset>=0.2.0 -> >=0.3.0 (configs.settings and configs.data_resolver were introduced in hydrodataset 0.3; the old floor allowed installing an incompatible 0.2.x)
The [tool.uv.sources] table points hydrodataset at a local editable path (../hydrodataset), which does not exist in CI runners. Add --no-sources so CI resolves hydrodataset>=0.3.0 from PyPI; local dev keeps the editable path.
uv run also reads [tool.uv.sources]; add --no-sources so it resolves the project from the already-synced PyPI environment instead of the local hydrodataset path that does not exist on CI runners.
netCDF4 1.7.2+ wheels bundle HDF5 1.14.6 which is incompatible with the h5py 3.15.x wheels, causing 'NetCDF: HDF error' when writing with to_netcdf on Linux CI. Per Unidata/netcdf4-python#1438, h5py 3.14.x is compatible with netCDF4 1.7.3.
tomllib is Python 3.11+; fall back to the tomli backport on 3.10 and add tomli as a conditional dev dependency so the pyproject assertion test collects on Python 3.10 (project supports 3.10+).
config: inject a storage default when the storage section is missing (S4); correct the lazy-load docstring to note module-level binds still trigger init (I1) data_resolver: docstring examples use registered dataset ids (S1); document that resolution requires a storage root and raises DatasetResolutionError without config (S2) data_source: add NetCDF attributes fallback for S3 datasets, guarding the unconfigured-S3 case (S3); drop the dead download param from LongTermDataset (S6); remove stale data_path comment (N1) utils: guard is_minio_folder against an unconfigured S3 filesystem (S5) floodevent: fix stray spaces in the time-range f-string (N2) readers: remove dead pathlib imports left by the migration (I2) tests: decouple the package-export count floor from hydrodataset's registry (N4); add regression tests for review cleanup, suggestions, and S3 fallback
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
6 participants
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
Align
hydrodatasourcewith the unified data path resolution contract (ADR 0001), reusinghydrodatasetas the resolver engine and exposing a one-lineopen_dataset()factory for downstream projects.hydrodatasource/configs/data_resolver.pyas a thin wrapper: injects HDSREADER_ALIASES+HDS_DATASETS, delegates to hydrodataseturi=positional)open_dataset(dataset_id)— resolves path + instantiates reader in one call_with_hds_extras()helper;resolve_data_pathnow usesResolverContextAPIopen_dataset,ResolverContext,READER_ALIASES,resolve_data_pathfrom package rootdatasets.yml; in-codeHDS_DATASETSis Layer 2, user YAML is Layer 3 overrideRelated
docs/spec-unified-data-interface.mddocs/adr/0001-unified-data-path-resolution.mdUsage (after merge)
Test plan
uv run pytest tests/test_data_resolver.py tests/test_package_exports.py— 37 passeduv run pytest tests/test_reader_uri_constructors.py— URI constructor testsopen_dataset(songliao_event)with valid~/hydro_setting.yml