Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal (and bugfix) to implement value-based conditional requirement #1511

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
845c49f
Varname mangling for value-based conditional req.
phargogh Jan 24, 2024
969ef23
Forgot a few docstrings. RE:#1503
phargogh Jan 24, 2024
273ef3f
Merge branch 'main' of https://github.com/natcap/invest into bugfix/1…
phargogh Jan 24, 2024
6096ad6
Noting change in HISTORY. RE:#1509
phargogh Jan 24, 2024
85d8028
Merge branch 'main' into bugfix/1509-una-validation-missing-uniform-s…
phargogh Feb 13, 2024
66f1490
Reworking sufficiency for clarity. RE:#1509
phargogh Feb 14, 2024
37a0338
Reworking comprehensions into standard loop for readability.
phargogh Feb 14, 2024
e1bef80
Reworking conditional requirement.
phargogh Feb 14, 2024
658748d
Removing .value in UNA conditional requirement.
phargogh Feb 14, 2024
0ce3303
Removing the variable name rewriting.
phargogh Feb 14, 2024
041f411
Slight refactor for efficiency. RE:#1509
phargogh Feb 14, 2024
4a1f548
First stab at rewriting nested cond. requirement.
phargogh Feb 14, 2024
416240d
Removing unnecessary conditional requirement - table already required…
phargogh Feb 14, 2024
21ae820
Clarifying varnames, copying spec to avoid modifying by reference, no…
phargogh Feb 14, 2024
b5a98a4
Merge branch 'bugfix/1509-una-validation-missing-uniform-search-radiu…
phargogh Feb 14, 2024
4135eec
Adding a test for vector field validity.
phargogh Feb 14, 2024
0b9111f
Adding a test for csv column conditional validity.
phargogh Feb 14, 2024
5e6e35b
Correcting WKT syntax. RE:#1509
phargogh Feb 14, 2024
b02b49c
Adding a test for CSV row conditional validation. RE:#1509
phargogh Feb 14, 2024
fec802a
Fixing expression in UCM. RE:#1509
phargogh Feb 14, 2024
f8497f6
Fixing operands in wind energy expression. RE:#1509
phargogh Feb 14, 2024
cb70a12
Removing a comment that is no longer relevant. RE:#1509
phargogh Feb 14, 2024
ecdc498
Switching bitwise operations to python bools. RE:#1509
phargogh Feb 14, 2024
bf55a26
Correcting boolean operator and->or. RE:#1509
phargogh Feb 14, 2024
482b327
Light linting, mostly around line length. RE:#1509
phargogh Feb 21, 2024
b32fcbf
Removing a deprecated line. RE:#1509
phargogh Feb 21, 2024
1deee3e
Simplifying validate(). RE:#1509
phargogh Feb 21, 2024
880e31f
Adding directory axis. RE:#1509
phargogh Feb 21, 2024
d70b15d
Casting to bool outside of the expression. RE:#1509
phargogh Feb 22, 2024
831fd99
Small test fix from slight return value change.
phargogh Feb 22, 2024
4b6f932
HQ validation now partially covered by expressions.
phargogh Feb 22, 2024
617e76d
Clarifying set names, restoring falsey exclusion.
phargogh Feb 22, 2024
03c7fb0
Noting changes in history.
phargogh Feb 27, 2024
c49c398
Merge branch 'main' into bugfix/1509-una-validation-missing-uniform-s…
phargogh Feb 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,28 @@

Unreleased Changes
------------------
* General
* We have updated validation in several ways that will improve the
developer experience of working with InVEST models, and we hope will also
improve the user experience:

* Symbols in conditional requirement expressions now represent the values
of parameters instead of whether the value of the parameter is
sufficient. If a symbol is not present in ``args`` as a key, the
symbol will have a value of ``False``. This allows for value-based
comparisons, which is useful in models that have overlapping modes of
operation. https://github.com/natcap/invest/issues/1509
* Vector fields, CSV rows/columns and the 1st level of directory
contents may now all be conditionally required based on a python
expression.
* Under certain circumstances, validation may return more warnings than
before. This specifically applies to model inputs that have conditional
requirement expressions where their expression evaluates to ``False``,
and the user has provided a value for this parameter. Previous
versions of InVEST would skip these parameters' type-specific
validation. Now, these parameters will be validated with their
type-specific validation checks.

* Annual Water Yield
* Added the results_suffix to a few intermediate files where it was
missing. https://github.com/natcap/invest/issues/1517
Expand All @@ -48,6 +70,9 @@ Unreleased Changes
* Fixed an issue where Urban Nature Access would crash if an administrative
boundary geometry did not overlap any people in the population raster.
https://github.com/natcap/invest/issues/1503
* Fixed an issue where validation was failing to catch missing values in
the uniform search radius args key when using uniform search radii.
https://github.com/natcap/invest/issues/1509
* Fixed an issue where the output administrative units vector's
``Pund_adm`` and ``Povr_adm`` fields representing undersupplied and
oversupplied populations, respectively, had values of 0 when running the
Expand Down
19 changes: 3 additions & 16 deletions src/natcap/invest/habitat_quality.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,22 @@
import os

import numpy
from osgeo import gdal
import pygeoprocessing
import taskgraph
from osgeo import gdal

from .model_metadata import MODEL_METADATA
from . import gettext
from . import spec_utils
from . import utils
from . import validation
from .model_metadata import MODEL_METADATA
from .unit_registry import u
from . import gettext


LOGGER = logging.getLogger(__name__)

MISSING_SENSITIVITY_TABLE_THREATS_MSG = gettext(
'Threats {threats} does not match any column in the sensitivity table. '
'Sensitivity columns: {column_names}') # (set of missing threats, set of found columns)
MISSING_COLUMN_MSG = gettext(
"The column '{column_name}' was not found in the Threat Data table for "
"the corresponding input LULC scenario.")
MISSING_THREAT_RASTER_MSG = gettext(
"A threat raster for threats: {threat_list} was not found or it "
"could not be opened by GDAL.")
Expand Down Expand Up @@ -1116,7 +1112,6 @@ def validate(args, limit_to=None):
bad_threat_paths = []
duplicate_paths = []
threat_path_list = []
bad_threat_columns = []
for lulc_key, lulc_arg in (('_c', 'lulc_cur_path'),
('_f', 'lulc_fut_path'),
('_b', 'lulc_bas_path')):
Expand All @@ -1126,9 +1121,6 @@ def validate(args, limit_to=None):
# threat_raster_folder
for threat, row in threat_df.iterrows():
threat_table_path_col = _THREAT_SCENARIO_MAP[lulc_key]
if threat_table_path_col not in row:
bad_threat_columns.append(threat_table_path_col)
break

# Threat path from threat CSV is relative to CSV
threat_path = row[threat_table_path_col]
Expand All @@ -1151,11 +1143,6 @@ def validate(args, limit_to=None):
duplicate_paths.append(
os.path.basename(threat_path))

if bad_threat_columns:
validation_warnings.append((
['threats_table_path'],
MISSING_COLUMN_MSG.format(column_name=bad_threat_columns[0])))

if bad_threat_paths:
validation_warnings.append((
['threats_table_path'],
Expand Down
6 changes: 3 additions & 3 deletions src/natcap/invest/urban_cooling_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,23 +71,23 @@
"considered a green area.")},
"shade": {
"type": "ratio",
"required": "cc_method == factors",
"required": "cc_method == 'factors'",
"about": gettext(
"The proportion of area in this LULC class that is "
"covered by tree canopy at least 2 meters high. "
"Required if the 'factors' option is selected for "
"the Cooling Capacity Calculation Method.")},
"albedo": {
"type": "ratio",
"required": "cc_method == factors",
"required": "cc_method == 'factors'",
"about": gettext(
"The proportion of solar radiation that is directly "
"reflected by this LULC class. Required if the "
"'factors' option is selected for the Cooling "
"Capacity Calculation Method.")},
"building_intensity": {
"type": "ratio",
"required": "cc_method == intensity",
"required": "cc_method == 'intensity'",
"about": gettext(
"The ratio of building floor area to footprint "
"area, with all values in this column normalized "
Expand Down
2 changes: 0 additions & 2 deletions src/natcap/invest/urban_nature_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,6 @@
'search_radius_m': {
'type': 'number',
'units': u.meter,
'required':
f'search_radius_mode == "{RADIUS_OPT_POP_GROUP}"',
'expression': 'value >= 0',
'about': gettext(
"The search radius in meters to use "
Expand Down
Loading
Loading