Description
Several places in the codebase use assert statements to validate user-provided inputs (config values, dataset coordinates). Python's assert statements are disabled when running with the -O (optimize) flag, which means these checks silently disappear in optimized deployments — potentially leading to data corruption or cryptic downstream errors instead of clear, early failures.
Affected locations
| File |
Line |
Statement |
Risk if skipped |
mllam_data_prep/ops/selection.py |
L70 |
assert sel_start != sel_end |
Equal start/end silently creates an empty slice |
mllam_data_prep/ops/selection.py |
L82–84 |
assert len(ds[coord]) > 0 |
Empty coordinate selection goes undetected |
mllam_data_prep/ops/cropping.py |
L56 |
assert da_lat.dims == da_lon.dims |
Mismatched lat/lon dims produce a wrong convex hull mask |
mllam_data_prep/ops/cropping.py |
L57 |
assert da_lat_ref.dims == da_lon_ref.dims |
Same for the reference dataset |
mllam_data_prep/config.py |
L494 |
assert args.f.endswith(".yaml") |
Non-YAML file silently passes validation |
Proposed fix
Replace each assert with an if … raise ValueError(…) that includes a descriptive message with the actual values involved. For example:
# before
assert sel_start != sel_end, "Start and end cannot be the same"
# after
if sel_start == sel_end:
raise ValueError(
f"Start and end of range selection for coordinate "
f"'{coord}' cannot be the same (got {sel_start!r})"
)
The same pattern already exists elsewhere in the codebase (e.g. the raise ValueError checks in selection.py L63–65 and stacking.py L31–34), so this would make error handling consistent.
Motivation
python -O is commonly used in production and containerised deployments for performance.
- The Python docs explicitly warn against using
assert for data validation.
- The sister repo
neural-lam tracks an equivalent issue (neural-lam#278).
Scope
This is a small, focused change — 5 assert → if/raise replacements, plus corresponding tests to verify that a ValueError is raised in each case. Happy to submit a PR if this sounds reasonable.
Description
Several places in the codebase use
assertstatements to validate user-provided inputs (config values, dataset coordinates). Python'sassertstatements are disabled when running with the-O(optimize) flag, which means these checks silently disappear in optimized deployments — potentially leading to data corruption or cryptic downstream errors instead of clear, early failures.Affected locations
mllam_data_prep/ops/selection.pyassert sel_start != sel_endmllam_data_prep/ops/selection.pyassert len(ds[coord]) > 0mllam_data_prep/ops/cropping.pyassert da_lat.dims == da_lon.dimsmllam_data_prep/ops/cropping.pyassert da_lat_ref.dims == da_lon_ref.dimsmllam_data_prep/config.pyassert args.f.endswith(".yaml")Proposed fix
Replace each
assertwith anif … raise ValueError(…)that includes a descriptive message with the actual values involved. For example:The same pattern already exists elsewhere in the codebase (e.g. the
raise ValueErrorchecks inselection.pyL63–65 andstacking.pyL31–34), so this would make error handling consistent.Motivation
python -Ois commonly used in production and containerised deployments for performance.assertfor data validation.neural-lamtracks an equivalent issue (neural-lam#278).Scope
This is a small, focused change — 5
assert→if/raisereplacements, plus corresponding tests to verify that aValueErroris raised in each case. Happy to submit a PR if this sounds reasonable.