Fix handling of bounded parameters - #63
Merged
pnorton-usgs merged 13 commits intoAug 14, 2026
Merged
Conversation
- Add 'is_bounded' flag in metadata when parameter min/max is 'bounded' - Resolve bounded parameter maximum to the actual dimension size in Parameter - Add bounded dimension to global dimensions in Parameters.add() - Remove post-load adjust_bounded_parameters() calls from ParameterFile, ParameterNetCDF, and ParamDb (now handled at parameter creation time) - Fix outlier detection to skip string-type parameters instead of checking for string min/max values - Use deepcopy for metadata and dimensions to prevent cross-contamination - Fix whitespace alignment in ParamDb
- Add test class ordering in conftest.py via pytest_collection_modifyitems - Add tests for bounded parameter creation with and without global_dims - Replace basin_solsta (bounded) with non-bounded params (ncol, maxiter_sntemp, gwstor_init) in tests that don't require bounded behavior - Use separate MetaData copies in test_ParameterFile to avoid shared state - Update expected output strings for new dimension/parameter additions
- Auto-set nhm_id from parameters when available in Cbh constructor - Fix cbh_files dict key to use string path instead of Path object - Fix resolve_units docstring formatting - Pass parameters to Cbh in netcdf test - Rename TestStreamflow class to TestDataFile in test_DataFile.py
The is_bounded check was inside the 'for cname in dimensions' loop, causing it to run on every iteration. For multi-dimension bounded parameters this would fail on the second iteration since maximum is already resolved to a numeric value. Move the resolution to run once after all dimensions are added.
The method is no longer called internally since bounded parameters are now resolved at creation time in Parameter.__init__. Emits a DeprecationWarning to inform any external callers.
Skip comparison for string-type parameters to avoid TypeError when comparing string data against numeric min/max bounds. Matches the guards already present in check_values() and outliers().
Clean up leftover comments from the old bounded handling approach in check() and check_values(). Remove unused default_val variable.
Previously, adding a bounded parameter that requires nobs would crash if npoigages was also missing from global dimensions. Now falls back to adding nobs with its metadata default size (0).
Raise ValueError if minimum > maximum (indicates corrupted metadata). Emit a warning when minimum == maximum (dimension size is zero, likely an unpopulated dimension).
Report partial outliers when only one bound is available rather than returning an empty list. Emit warnings when minimum, maximum, or both are undefined so the caller knows the check was incomplete.
…ng_of_bounded_parameters Incorporate latest development fixes
This was referenced Aug 17, 2026
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
1 participant
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
Refactors bounded parameter handling so that the maximum value (a dimension name in metadata) is resolved to a numeric size at parameter creation time rather than requiring a post-load adjustment pass.
Changes
Core bounded parameter handling
is_boundedflag when parameter min/max isbounded__init__, moved outside the dimension loop to avoid repeated execution for multi-dimension parametersadd(); remove post-loadadjust_bounded_parameters()calls (now deprecated with warning)adjust_bounded_parameters()calls; bounded resolution is now automaticValidation improvements
check_values()andoutliers(): Skip string-type parameters; raiseValueErrorif min > max; warn if min == maxoutlier_ids(): Guard against string datatype and None min/max; report partial outliers with warnings when only one bound is availableOther fixes
Cbh.py: Auto-setnhm_idfrom parameters; fix dict key type for cbh_filestest_DataFile.py: RenameTestStreamflowtoTestDataFileconftest.py: Add test class orderingParameters.add(): Fall back to metadata default fornobswhennpoigagesis missingdeepcopyfor metadata/dimensions to prevent cross-instance contaminationTesting
All 301 tests pass locally (Python 3.13).