Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates DISDRODB to a new archive/file naming version (V1), including a timestamp format change in filenames, and adjusts related tests/configs. It also includes a few plotting enhancements in the visualization utilities.
Changes:
- Bump
ARCHIVE_VERSIONtoV1and update filename time encoding toYYYYMMDDTHHMMSSacross path generation and filename parsing. - Update tests to reflect the new filename convention/version.
- Add minor viz improvements (axis limits, sample interval inference in quicklooks, empty-spectrum handling) and adjust LPM velocity bin definitions.
Reviewed changes
Copilot reviewed 13 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| disdrodb/constants.py | Bumps archive version to V1. |
| disdrodb/api/path.py | Updates filename time formatting to include T. |
| disdrodb/api/info.py | Updates trollsift filename patterns to match new timestamp format. |
| disdrodb/viz/plots.py | Adds axis limit options, infers sample_interval in quicklooks, and tweaks spectrum plotting behavior. |
| disdrodb/l0/configs/LPM/bins_velocity.yml | Adjusts last velocity bin center/bounds/width. |
| disdrodb/l0/configs/LPM_V0/bins_velocity.yml | Same velocity-bin adjustment for the V0 config directory. |
| disdrodb/tests/test_utils/test_utils_archiving.py | Updates expected filenames/version handling for V1. |
| disdrodb/tests/test_routines/test_options.py | Updates test fixtures to the new timestamp format. |
| disdrodb/tests/test_routines/test_cmd_l1_processing.py | Updates commented example to use ARCHIVE_VERSION. |
| disdrodb/tests/test_l0/test_check_standards.py | Updates expected L0A filename to V1 + T timestamp format. |
| disdrodb/tests/test_api/test_api_path.py | Updates expected filenames to V1 + T timestamp format. |
| disdrodb/tests/test_api/test_api_io.py | Updates test filenames used for discovery/opening/filtering to V1 + T timestamp format. |
| disdrodb/tests/test_api/test_api_info.py | Updates start/end time strings/parsing but still has a version mismatch (see comments). |
| disdrodb/tests/data/check_readers/**.V1.nc | Adds/updates V1 reader ground-truth netCDF fixture. |
| disdrodb/tests/data/check_readers/**.V1.parquet | Adds/updates V1 reader ground-truth parquet fixture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "start_time": "20180625T004331", | ||
| "end_time": "20180711T010000", | ||
| "version": "1", | ||
| "data_format": "parquet", | ||
| } |
There was a problem hiding this comment.
FILE_INFO["version"] is still set to "1", but the filename patterns (and other tests) use ARCHIVE_VERSION values like "V1". This makes VALID_FNAME inconsistent with the new V1 naming scheme and can cause parsing/assertions to fail. Use ARCHIVE_VERSION (or set the dict to "V1") for the version field to match the filenames being generated/parsed.
| # Set array all to NaN if no particles to avoid plotting empty spectrum with vmin color | ||
| n_particles = drop_number.sum().item() | ||
| if n_particles == 0: | ||
| drop_number.data[:] = np.nan | ||
|
|
There was a problem hiding this comment.
The empty-spectrum handling has two issues: (1) norm = LogNorm(...) if drop_number.sum() > 0 else None will evaluate a 0-d xarray DataArray in a boolean context (can raise “truth value of a DataArray is ambiguous”); use a scalar like drop_number.sum().item() once and reuse it. (2) drop_number.data[:] = np.nan mutates the DataArray in-place; if the input is already in memory, this can unexpectedly modify the caller’s dataset. Prefer a non-mutating approach (e.g., drop_number = drop_number.where(False) / .where(drop_number.notnull(), np.nan) or drop_number = drop_number.copy() before assignment).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #251 +/- ##
==========================================
- Coverage 91.09% 90.40% -0.69%
==========================================
Files 122 123 +1
Lines 13099 13501 +402
==========================================
+ Hits 11932 12206 +274
- Misses 1167 1295 +128 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
coverage: 90.335% (-0.7%) from 91.014% — ghiggi:toward-v1 into ltelab:main |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 1 minor |
| Documentation | 10 minor |
| Security | 9 high |
🟢 Metrics 92 complexity · -7 duplication
Metric Results Complexity 92 Duplication -7
TIP This summary will be updated as you push new changes. Give us feedback
Prework
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
This PR change file archive version to V1. V0 files can't be read anymore with this disdrodb version !