From f22fc085354ca09419e9e69c458dab6dbcc40ba4 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Wed, 17 Jan 2024 17:18:51 +0100 Subject: [PATCH 01/22] Update pyproject to correct schema version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f172c006..e2b10e0c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ dependencies = [ "Click", "GeoAlchemy2>=0.9,!=0.11.*", "SQLAlchemy>=1.4", - "threedi-schema==0.217.*", + "threedi-schema==0.218.*", ] [project.optional-dependencies] From 5f16b37456c30e1550365814eaecb60de820170c Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Wed, 17 Jan 2024 17:59:14 +0100 Subject: [PATCH 02/22] Add check if plural parameters content can be converted to list of floats --- threedi_modelchecker/config.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 2da9d5ff..90cfcd11 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -653,7 +653,22 @@ def is_none_or_empty(col): level=CheckLevel.WARNING, ), ] - +CHECKS += [ + CrossSectionFloatListCheck( + error_code=87, + column=col, + shapes=( + constants.CrossSectionShape.TABULATED_YZ, + ), + ) + for col in [ + models.CrossSectionDefinition.friction_values, + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, + ] +] ## 01xx: LEVEL CHECKS From 15d73a895d7d732b9d4ed3271f46195cec566f9c Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Wed, 17 Jan 2024 17:59:14 +0100 Subject: [PATCH 03/22] Add check if plural parameters content can be converted to list of floats --- threedi_modelchecker/config.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 2da9d5ff..2b68f754 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2754,6 +2754,24 @@ def is_none_or_empty(col): ) ] +## 018x cross section parameters (continues 008x) +CHECKS += [ + CrossSectionFloatListCheck( + error_code=187, + column=col, + shapes=( + constants.CrossSectionShape.TABULATED_YZ, + ), + ) + for col in [ + models.CrossSectionDefinition.friction_values, + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, + ] +] + # These checks are optional, depending on a command line argument beta_features_check = [] beta_features_check += [ From ec23912e1fd3b2a430843f59634fb1bddf217525 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Thu, 18 Jan 2024 10:02:09 +0100 Subject: [PATCH 04/22] Add check for correct lengths of cross section variables --- .../checks/cross_section_definitions.py | 24 +++++++++++ threedi_modelchecker/config.py | 43 ++++++++++++++----- .../test_checks_cross_section_definitions.py | 14 ++++++ 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index 3a68fde7..eaaa771b 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -583,3 +583,27 @@ def description(self): "of friction is recommended in case there is a significant variation " "of the bed level (for instance, in a scenario with overflowing floodplains)." ) + + +class CrossSectionVariableCorrectLengthCheck(CrossSectionBaseCheck): + """Variable friction and vegetation properties should 1 value for each element; len(var_property) = len(width)-1""" + + def get_invalid(self, session): + invalids = [] + for record in self.to_check(session).filter( + (self.column.name != None) & (self.column.name != "") + ): + try: + # only take widths because another check already ensures len(widths) = len(heights) + widths = [float(x) for x in record.width.split(" ")] + values = [ + float(x) for x in getattr(record, self.column.name).split(" ") + ] + except ValueError: + continue # other check catches this + if not (len(widths) - 1 == len(values)): + invalids.append(record) + return invalids + + def description(self): + return f"{self.column.name} should contain exactly 1 value less as the profile" diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 2b68f754..3fa1068a 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -28,6 +28,7 @@ CrossSectionIncreasingCheck, CrossSectionMinimumDiameterCheck, CrossSectionNullCheck, + CrossSectionVariableCorrectLengthCheck, CrossSectionYZCoordinateCountCheck, CrossSectionYZHeightCheck, CrossSectionYZIncreasingWidthIfOpenCheck, @@ -2755,23 +2756,45 @@ def is_none_or_empty(col): ] ## 018x cross section parameters (continues 008x) +par_cols = [ + models.CrossSectionDefinition.friction_values, + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, +] +CHECKS += [ + QueryCheck( + error_code=180, + column=col, + invalid=Query(models.CrossSectionDefinition) + .filter( + models.CrossSectionDefinition.shape + != constants.CrossSectionShape.TABULATED_YZ + ) + .filter(col.is_not(None)), + message=(f"{col} can only be used in combination with a TABULATED_YZ"), + ) + for col in par_cols +] +CHECKS += [ + CrossSectionVariableCorrectLengthCheck( + error_code=181, + column=col, + shapes=(constants.CrossSectionShape.TABULATED_YZ,), + ) + for col in par_cols +] CHECKS += [ CrossSectionFloatListCheck( error_code=187, column=col, - shapes=( - constants.CrossSectionShape.TABULATED_YZ, - ), + shapes=(constants.CrossSectionShape.TABULATED_YZ,), ) - for col in [ - models.CrossSectionDefinition.friction_values, - models.CrossSectionDefinition.vegetation_drag_coefficients, - models.CrossSectionDefinition.vegetation_heights, - models.CrossSectionDefinition.vegetation_stem_diameters, - models.CrossSectionDefinition.vegetation_stem_densities, - ] + for col in par_cols ] + # These checks are optional, depending on a command line argument beta_features_check = [] beta_features_check += [ diff --git a/threedi_modelchecker/tests/test_checks_cross_section_definitions.py b/threedi_modelchecker/tests/test_checks_cross_section_definitions.py index 527e1738..fcc2dadc 100644 --- a/threedi_modelchecker/tests/test_checks_cross_section_definitions.py +++ b/threedi_modelchecker/tests/test_checks_cross_section_definitions.py @@ -12,6 +12,7 @@ CrossSectionIncreasingCheck, CrossSectionMinimumDiameterCheck, CrossSectionNullCheck, + CrossSectionVariableCorrectLengthCheck, CrossSectionYZCoordinateCountCheck, CrossSectionYZHeightCheck, CrossSectionYZIncreasingWidthIfOpenCheck, @@ -683,3 +684,16 @@ def test_check_cross_section_conveyance_friction_info_message( expected_result = 0 invalid_rows = check.get_invalid(session) assert len(invalid_rows) == expected_result + + +@pytest.mark.parametrize("data, result", [["1 2", True], ["1 2 3", False]]) +def test_check_correct_length(session, data, result): + definition = factories.CrossSectionDefinitionFactory( + width="1 2 3", height="0 2 5", friction_values=data + ) + factories.CrossSectionLocationFactory(definition=definition) + check = CrossSectionVariableCorrectLengthCheck( + column=models.CrossSectionDefinition.friction_values + ) + invalid_rows = check.get_invalid(session) + assert (len(invalid_rows) == 0) == result From aa9936a702b8599cbac9d094f6fd494dcfcc68ef Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Fri, 19 Jan 2024 13:24:23 +0100 Subject: [PATCH 05/22] Add range checks --- .../checks/cross_section_definitions.py | 94 +++++++++++++++++++ threedi_modelchecker/config.py | 61 ++++++++++++ .../test_checks_cross_section_definitions.py | 55 +++++++++++ 3 files changed, 210 insertions(+) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index eaaa771b..715ebf30 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -607,3 +607,97 @@ def get_invalid(self, session): def description(self): return f"{self.column.name} should contain exactly 1 value less as the profile" + + +class CrossSectionVariableRangeCheck(CrossSectionBaseCheck): + def __init__( + self, + min_value=None, + max_value=None, + left_inclusive=True, + right_inclusive=True, + *args, + **kwargs, + ): + if min_value is None and max_value is None: + raise ValueError("Please supply at least one of {min_value, max_value}.") + str_parts = [] + if min_value is None: + self.min_valid = lambda x: True + else: + self.min_valid = ( + (lambda x: x >= min_value) + if left_inclusive + else (lambda x: x > min_value) + ) + str_parts.append(f"{'< ' if left_inclusive else '<= '}{min_value}") + if max_value is None: + self.max_valid = lambda x: True + else: + self.max_valid = ( + (lambda x: x <= max_value) + if right_inclusive + else (lambda x: x < max_value) + ) + str_parts.append(f"{'> ' if right_inclusive else '>= '}{max_value}") + self.range_str = " and/or ".join(str_parts) + super().__init__(*args, **kwargs) + + def get_invalid(self, session): + invalids = [] + for record in self.to_check(session).filter( + (self.column != None) & (self.column != "") + ): + try: + values = [ + float(x) for x in getattr(record, self.column.name).split(" ") + ] + except ValueError: + invalids.append(record) + if not self.min_valid(min(values)): + invalids.append(record) + elif not self.max_valid(max(values)): + invalids.append(record) + return invalids + + def description(self): + return f"some values in {self.column_name} are {self.range_str}" + + +class CrossSectionVariableFrictionRangeCheck(CrossSectionVariableRangeCheck): + def __init__( + self, + friction_types, + *args, + **kwargs, + ): + self.friction_types = friction_types + super().__init__(*args, **kwargs) + + def get_invalid(self, session): + invalids = [] + def_table = models.CrossSectionDefinition + loc_table = models.CrossSectionLocation + print(self.friction_types) + records = set( + self.to_check(session) + .join(loc_table, loc_table.definition_id == def_table.id) + .filter( + loc_table.friction_type.in_(self.friction_types) + & def_table.friction_values.is_not(None) + ) + .filter((self.column != None) & (self.column != "")) + .all() + ) + for record in records: + try: + values = [ + float(x) for x in getattr(record, self.column.name).split(" ") + ] + except ValueError: + continue + if not self.min_valid(min(values)): + invalids.append(record) + elif not self.max_valid(max(values)): + invalids.append(record) + return invalids diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 3fa1068a..e56a0f39 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -29,6 +29,8 @@ CrossSectionMinimumDiameterCheck, CrossSectionNullCheck, CrossSectionVariableCorrectLengthCheck, + CrossSectionVariableFrictionRangeCheck, + CrossSectionVariableRangeCheck, CrossSectionYZCoordinateCountCheck, CrossSectionYZHeightCheck, CrossSectionYZIncreasingWidthIfOpenCheck, @@ -2782,6 +2784,7 @@ def is_none_or_empty(col): error_code=181, column=col, shapes=(constants.CrossSectionShape.TABULATED_YZ,), + filters=models.CrossSectionDefinition.height.is_not(None) & col.is_not(None), ) for col in par_cols ] @@ -2794,6 +2797,64 @@ def is_none_or_empty(col): for col in par_cols ] +## Friction values - move - give correct number +## 9999 +CHECKS += [ + CrossSectionVariableFrictionRangeCheck( + min_value=0, + max_value=1, + right_inclusive=False, + error_code=9999, + column=models.CrossSectionDefinition.friction_values, + shapes=(constants.CrossSectionShape.TABULATED_YZ,), + friction_types=[ + constants.FrictionType.MANNING.value, + constants.FrictionType.MANNING_CONVEYANCE.value, + ], + ) +] +CHECKS += [ + CrossSectionVariableFrictionRangeCheck( + min_value=0, + error_code=9999, + column=models.CrossSectionDefinition.friction_values, + shapes=(constants.CrossSectionShape.TABULATED_YZ,), + friction_types=[ + constants.FrictionType.CHEZY.value, + constants.FrictionType.CHEZY_CONVEYANCE.value, + ], + ) +] + +## 019x vegetation parameter checks +CHECKS += [ + RangeCheck( + error_code=190, + column=col, + min_value=0, + ) + for col in [ + models.CrossSectionLocation.vegetation_drag_coefficient, + models.CrossSectionLocation.vegetation_height, + models.CrossSectionLocation.vegetation_stem_diameter, + models.CrossSectionLocation.vegetation_stem_density, + ] +] +CHECKS += [ + CrossSectionVariableRangeCheck( + error_code=190, + min_value=0, + column=col, + shapes=(constants.CrossSectionShape.TABULATED_YZ,), + ) + for col in [ + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, + ] +] + # These checks are optional, depending on a command line argument beta_features_check = [] diff --git a/threedi_modelchecker/tests/test_checks_cross_section_definitions.py b/threedi_modelchecker/tests/test_checks_cross_section_definitions.py index fcc2dadc..feadf69f 100644 --- a/threedi_modelchecker/tests/test_checks_cross_section_definitions.py +++ b/threedi_modelchecker/tests/test_checks_cross_section_definitions.py @@ -13,6 +13,8 @@ CrossSectionMinimumDiameterCheck, CrossSectionNullCheck, CrossSectionVariableCorrectLengthCheck, + CrossSectionVariableFrictionRangeCheck, + CrossSectionVariableRangeCheck, CrossSectionYZCoordinateCountCheck, CrossSectionYZHeightCheck, CrossSectionYZIncreasingWidthIfOpenCheck, @@ -697,3 +699,56 @@ def test_check_correct_length(session, data, result): ) invalid_rows = check.get_invalid(session) assert (len(invalid_rows) == 0) == result + + +@pytest.mark.parametrize( + "min_value, max_value, left_incl, right_incl, result", + [ + [0, 1, True, True, True], + [0, 0.5, True, True, False], + [0.5, 1, True, True, False], + [0, 1, False, True, False], + [0, 1, True, False, False], + [0, None, True, True, True], + [None, 1, True, True, True], + ], +) +def test_check_var_range(session, min_value, max_value, left_incl, right_incl, result): + definition = factories.CrossSectionDefinitionFactory(friction_values="0 1") + factories.CrossSectionLocationFactory(definition=definition) + check = CrossSectionVariableRangeCheck( + column=models.CrossSectionDefinition.friction_values, + min_value=min_value, + max_value=max_value, + left_inclusive=left_incl, + right_inclusive=right_incl, + ) + invalid_rows = check.get_invalid(session) + assert (len(invalid_rows) == 0) == result + + +@pytest.mark.parametrize( + "friction_types, result", + [ + [[constants.FrictionType.MANNING], False], + [[constants.FrictionType.CHEZY], True], + ], +) +def test_check_friction_values_range(session, friction_types, result): + definition = factories.CrossSectionDefinitionFactory(friction_values="0 2") + factories.CrossSectionLocationFactory( + definition=definition, friction_type=constants.FrictionType.MANNING + ) + check = CrossSectionVariableFrictionRangeCheck( + min_value=0, + max_value=1, + right_inclusive=False, + error_code=9999, + column=models.CrossSectionDefinition.friction_values, + friction_types=friction_types, + ) + invalid_rows = check.get_invalid(session) + for row in invalid_rows: + print(type(row)) + print(dir(row)) + assert (len(invalid_rows) == 0) == result From 1eb53176fa0dced7273ecd7256f364404c138231 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Fri, 19 Jan 2024 14:17:00 +0100 Subject: [PATCH 06/22] Check for friction type combined with vegetation --- .../checks/cross_section_definitions.py | 1 - threedi_modelchecker/config.py | 66 ++++++++++++++++--- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index 715ebf30..ea15dfbf 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -678,7 +678,6 @@ def get_invalid(self, session): invalids = [] def_table = models.CrossSectionDefinition loc_table = models.CrossSectionLocation - print(self.friction_types) records = set( self.to_check(session) .join(loc_table, loc_table.definition_id == def_table.id) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 8f70ec52..de861a3e 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -660,9 +660,7 @@ def is_none_or_empty(col): CrossSectionFloatListCheck( error_code=87, column=col, - shapes=( - constants.CrossSectionShape.TABULATED_YZ, - ), + shapes=(constants.CrossSectionShape.TABULATED_YZ,), ) for col in [ models.CrossSectionDefinition.friction_values, @@ -2773,8 +2771,7 @@ def is_none_or_empty(col): ] ## 018x cross section parameters (continues 008x) -par_cols = [ - models.CrossSectionDefinition.friction_values, +veg_par_cols = [ models.CrossSectionDefinition.vegetation_drag_coefficients, models.CrossSectionDefinition.vegetation_heights, models.CrossSectionDefinition.vegetation_stem_diameters, @@ -2792,7 +2789,7 @@ def is_none_or_empty(col): .filter(col.is_not(None)), message=(f"{col} can only be used in combination with a TABULATED_YZ"), ) - for col in par_cols + for col in veg_par_cols ] CHECKS += [ CrossSectionVariableCorrectLengthCheck( @@ -2801,7 +2798,7 @@ def is_none_or_empty(col): shapes=(constants.CrossSectionShape.TABULATED_YZ,), filters=models.CrossSectionDefinition.height.is_not(None) & col.is_not(None), ) - for col in par_cols + for col in veg_par_cols ] CHECKS += [ CrossSectionFloatListCheck( @@ -2809,7 +2806,7 @@ def is_none_or_empty(col): column=col, shapes=(constants.CrossSectionShape.TABULATED_YZ,), ) - for col in par_cols + for col in veg_par_cols ] ## Friction values - move - give correct number @@ -2870,6 +2867,59 @@ def is_none_or_empty(col): ] ] +CHECKS += [ + QueryCheck( + error_code=191, + column=col, + invalid=Query(models.CrossSectionLocation) + .filter( + models.CrossSectionLocation.friction_type.in_( + [ + constants.FrictionType.MANNING, + constants.FrictionType.MANNING_CONVEYANCE, + ] + ) + ) + .filter(col.is_not(None)), + message=(f"{col} cannot be used with Manning type friction"), + ) + for col in [ + models.CrossSectionLocation.vegetation_drag_coefficient, + models.CrossSectionLocation.vegetation_height, + models.CrossSectionLocation.vegetation_stem_diameter, + models.CrossSectionLocation.vegetation_stem_density, + ] +] +CHECKS += [ + QueryCheck( + error_code=191, + column=col, + invalid=( + Query(models.CrossSectionDefinition) + .join( + models.CrossSectionLocation, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter( + models.CrossSectionLocation.friction_type.in_( + [ + constants.FrictionType.MANNING, + constants.FrictionType.MANNING_CONVEYANCE, + ] + ) + & col.is_not(None) + ) + ), + message=(f"{col} cannot be used with Manning type friction"), + ) + for col in [ + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, + ] +] # These checks are optional, depending on a command line argument beta_features_check = [] From d44bbe4590a5941f1a8abaf49bbad50c46252570 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Fri, 19 Jan 2024 14:44:57 +0100 Subject: [PATCH 07/22] Check for friction type and vegetation parameters --- threedi_modelchecker/config.py | 78 ++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index de861a3e..8875472f 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2808,6 +2808,84 @@ def is_none_or_empty(col): ) for col in veg_par_cols ] +CHECKS += [ + QueryCheck( + error_code=188, + level=CheckLevel.WARNING, + column=col_csloc, + invalid=Query(models.CrossSectionDefinition) + .join( + models.CrossSectionLocation, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter(col_csloc.is_not(None) & col_csdef.is_not(None)) + .filter( + models.CrossSectionLocation.friction_type.is_(constants.FrictionType.CHEZY) + ), + message=( + f"Both {col_csloc} and {col_csdef} defined without conveyane; {col_csloc} will be used" + ), + ) + for col_csloc, col_csdef in [ + ( + models.CrossSectionLocation.vegetation_drag_coefficient, + models.CrossSectionDefinition.vegetation_drag_coefficients, + ), + ( + models.CrossSectionLocation.vegetation_height, + models.CrossSectionDefinition.vegetation_heights, + ), + ( + models.CrossSectionLocation.vegetation_stem_diameter, + models.CrossSectionDefinition.vegetation_stem_diameters, + ), + ( + models.CrossSectionLocation.vegetation_stem_density, + models.CrossSectionDefinition.vegetation_stem_densities, + ), + ] +] +CHECKS += [ + QueryCheck( + error_code=188, + level=CheckLevel.WARNING, + column=col_csloc, + invalid=Query(models.CrossSectionDefinition) + .join( + models.CrossSectionLocation, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter(col_csloc.is_not(None) & col_csdef.is_not(None)) + .filter( + models.CrossSectionLocation.friction_type.is_( + constants.FrictionType.CHEZY_CONVEYANCE + ) + ), + message=( + f"Both {col_csloc} and {col_csdef} defined with conveyane; {col_csdef} will be used" + ), + ) + for col_csloc, col_csdef in [ + ( + models.CrossSectionLocation.vegetation_drag_coefficient, + models.CrossSectionDefinition.vegetation_drag_coefficients, + ), + ( + models.CrossSectionLocation.vegetation_height, + models.CrossSectionDefinition.vegetation_heights, + ), + ( + models.CrossSectionLocation.vegetation_stem_diameter, + models.CrossSectionDefinition.vegetation_stem_diameters, + ), + ( + models.CrossSectionLocation.vegetation_stem_density, + models.CrossSectionDefinition.vegetation_stem_densities, + ), + ] +] ## Friction values - move - give correct number ## 9999 From 70f93938dcd79f556c8f96a7e5ad62207cc8980d Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 22 Jan 2024 10:55:08 +0100 Subject: [PATCH 08/22] Adapt tests to allow for tabulated yz shapes without friction value and catch tabulated yz shapes with neither friction value or friction values defined --- threedi_modelchecker/config.py | 52 ++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 8875472f..e419aa43 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -168,6 +168,27 @@ def is_none_or_empty(col): CHECKS: List[BaseCheck] = [] ## 002x: FRICTION +## Use same error code as other null checks +CHECKS += [ + QueryCheck( + error_code=3, + column=models.CrossSectionLocation.friction_value, + invalid=( + Query(models.CrossSectionLocation) + .join( + models.CrossSectionDefinition, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter( + models.CrossSectionDefinition.shape + != constants.CrossSectionShape.TABULATED_YZ + ) + .filter(models.CrossSectionLocation.friction_value == None) + ), + message=f"{models.CrossSectionLocation.friction_value.name} cannot be null or empty", + ) +] CHECKS += [ RangeCheck( error_code=21, @@ -532,7 +553,30 @@ def is_none_or_empty(col): ] ## 008x: CROSS SECTION DEFINITIONS - +CHECKS += [ + QueryCheck( + error_code=80, + column=models.CrossSectionLocation.friction_value, + invalid=( + Query(models.CrossSectionDefinition) + .filter( + models.CrossSectionDefinition.shape + == constants.CrossSectionShape.TABULATED_YZ + ) + .join( + models.CrossSectionLocation, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter(models.CrossSectionLocation.friction_value == None) + .filter( + (models.CrossSectionDefinition.friction_values == None) + | (models.CrossSectionDefinition.friction_values == "") + ) + ), + message="Either friction value or friction values must be defined for a TABULATED YZ shape", + ) +] CHECKS += [ CrossSectionNullCheck( error_code=81, @@ -2770,6 +2814,8 @@ def is_none_or_empty(col): ) ] +# TODO: reconsider number because 01xx exists! +# TODO add friction value / friction_values check here ## 018x cross section parameters (continues 008x) veg_par_cols = [ models.CrossSectionDefinition.vegetation_drag_coefficients, @@ -2810,7 +2856,7 @@ def is_none_or_empty(col): ] CHECKS += [ QueryCheck( - error_code=188, + error_code=182, level=CheckLevel.WARNING, column=col_csloc, invalid=Query(models.CrossSectionDefinition) @@ -2848,7 +2894,7 @@ def is_none_or_empty(col): ] CHECKS += [ QueryCheck( - error_code=188, + error_code=182, level=CheckLevel.WARNING, column=col_csloc, invalid=Query(models.CrossSectionDefinition) From 7dd9154d8353d094269b1c899b6d52a3883e50a7 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 22 Jan 2024 11:51:05 +0100 Subject: [PATCH 09/22] Add warnings for cases where both friction_value and friction_values are defined --- threedi_modelchecker/config.py | 60 ++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index e419aa43..b6ad2cb6 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2932,6 +2932,66 @@ def is_none_or_empty(col): ), ] ] +CHECKS += [ + QueryCheck( + error_code=183, + level=CheckLevel.WARNING, + column=models.CrossSectionDefinition.friction_values, + invalid=( + Query(models.CrossSectionDefinition) + .join( + models.CrossSectionLocation, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter( + ( + models.CrossSectionLocation.friction_type + == constants.FrictionType.CHEZY_CONVEYANCE + ) + | ( + models.CrossSectionLocation.friction_type + == constants.FrictionType.MANNING_CONVEYANCE + ) + ) + .filter( + models.CrossSectionDefinition.friction_values.is_not(None) + & models.CrossSectionLocation.friction_value.is_not(None) + ) + ), + message="Both {friction_value} and {friction_values} are defined for conveyance friction. " + "Only {friction_values} will be used.", + ), + QueryCheck( + error_code=183, + level=CheckLevel.WARNING, + column=models.CrossSectionDefinition.friction_values, + invalid=( + Query(models.CrossSectionDefinition) + .join( + models.CrossSectionLocation, + models.CrossSectionLocation.definition_id + == models.CrossSectionDefinition.id, + ) + .filter( + ( + models.CrossSectionLocation.friction_type + == constants.FrictionType.CHEZY + ) + | ( + models.CrossSectionLocation.friction_type + == constants.FrictionType.MANNING + ) + ) + .filter( + models.CrossSectionDefinition.friction_values.is_not(None) + & models.CrossSectionLocation.friction_value.is_not(None) + ) + ), + message="Both {friction_value} and {friction_values} are defined for non-conveyance friction. " + "Only {friction_value} will be used.", + ), +] ## Friction values - move - give correct number ## 9999 From be2210d5e94324754f74dfef64fa1466280f20d1 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 22 Jan 2024 14:10:25 +0100 Subject: [PATCH 10/22] Add checks for allowed profile shape --- .../checks/cross_section_definitions.py | 45 +++++++++++++++++++ threedi_modelchecker/config.py | 8 ++++ .../test_checks_cross_section_definitions.py | 43 ++++++++++++++++-- 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index ea15dfbf..2aabcedc 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -700,3 +700,48 @@ def get_invalid(self, session): elif not self.max_valid(max(values)): invalids.append(record) return invalids + + +class OpenIncreasingCrossSectionVariableCheck(CrossSectionBaseCheck): + """ + Check if cross sections used with friction with conveyance + are open and monotonically increasing in width + """ + + def __init__(self, *args, **kwargs): + super().__init__( + shapes=(constants.CrossSectionShape.TABULATED_YZ,), *args, **kwargs + ) + + def get_invalid(self, session): + invalids = [] + records = self.to_check(session).filter( + (self.column != None) & (self.column != "") + ) + for record in records: + try: + # Only used for TABULATED_YZ + widths = [float(x) for x in record.width.split(" ")] + heights = [float(x) for x in record.height.split(" ")] + except ValueError: + continue # other check catches this + + _, _, configuration = cross_section_configuration( + shape=record.shape.value, heights=heights, widths=widths + ) + + # friction with conveyance can only be used for cross-sections + # which are open *and* have a monotonically increasing width + if configuration == "closed" or ( + len(widths) > 1 + and any( + next_width < previous_width + for (previous_width, next_width) in zip(widths[:-1], widths[1:]) + ) + ): + invalids.append(record) + + return invalids + + def description(self): + return f"{self.column_name} can only be used in an open channel with monotonically increasing width values" diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index b6ad2cb6..cf9c4b07 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -35,6 +35,7 @@ CrossSectionYZHeightCheck, CrossSectionYZIncreasingWidthIfOpenCheck, OpenIncreasingCrossSectionConveyanceFrictionCheck, + OpenIncreasingCrossSectionVariableCheck, ) from .checks.factories import ( generate_enum_checks, @@ -2992,6 +2993,13 @@ def is_none_or_empty(col): "Only {friction_value} will be used.", ), ] +CHECKS += [ + OpenIncreasingCrossSectionVariableCheck( + error_code=184, + column=col, + ) + for col in veg_par_cols + [models.CrossSectionDefinition.friction_values] +] ## Friction values - move - give correct number ## 9999 diff --git a/threedi_modelchecker/tests/test_checks_cross_section_definitions.py b/threedi_modelchecker/tests/test_checks_cross_section_definitions.py index feadf69f..70f83357 100644 --- a/threedi_modelchecker/tests/test_checks_cross_section_definitions.py +++ b/threedi_modelchecker/tests/test_checks_cross_section_definitions.py @@ -19,6 +19,7 @@ CrossSectionYZHeightCheck, CrossSectionYZIncreasingWidthIfOpenCheck, OpenIncreasingCrossSectionConveyanceFrictionCheck, + OpenIncreasingCrossSectionVariableCheck, ) from . import factories @@ -748,7 +749,43 @@ def test_check_friction_values_range(session, friction_types, result): friction_types=friction_types, ) invalid_rows = check.get_invalid(session) - for row in invalid_rows: - print(type(row)) - print(dir(row)) + assert (len(invalid_rows) == 0) == result + + +@pytest.mark.parametrize( + "width,height,result", + [ + ( + "0.01 0.11", + "0.11 0.21", + True, + ), # open tabulated yz, increasing width, pass + ( + "0.11 0.01", + "0.11 0.20", + False, + ), # open tabulated yz, decreasing width, fail + ( + "0.01 0.11 0.01", + "0.11 0.21 0.11", + False, + ), # closed tabulated yz, fail + ], +) +def test_check_cross_section_increasing_open_with_variables( + session, width, height, result +): + definition = factories.CrossSectionDefinitionFactory( + shape=constants.CrossSectionShape.TABULATED_YZ, + width=width, + height=height, + friction_values="1", + ) + factories.CrossSectionLocationFactory(definition=definition) + check = OpenIncreasingCrossSectionVariableCheck( + models.CrossSectionDefinition.friction_values + ) + # this check should pass on cross-section locations which don't use conveyance, + # regardless of their other parameters + invalid_rows = check.get_invalid(session) assert (len(invalid_rows) == 0) == result From fe1dcf18d1d35cfa4112d2c5b108df64b4056a4f Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Tue, 23 Jan 2024 11:32:13 +0100 Subject: [PATCH 11/22] Clean up error messages --- .../checks/cross_section_definitions.py | 2 +- threedi_modelchecker/config.py | 39 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index 2aabcedc..3a4f84ff 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -606,7 +606,7 @@ def get_invalid(self, session): return invalids def description(self): - return f"{self.column.name} should contain exactly 1 value less as the profile" + return f"{self.column_name} should contain exactly 1 value less as the lenght and width" class CrossSectionVariableRangeCheck(CrossSectionBaseCheck): diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index cf9c4b07..3d7c89df 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -187,7 +187,7 @@ def is_none_or_empty(col): ) .filter(models.CrossSectionLocation.friction_value == None) ), - message=f"{models.CrossSectionLocation.friction_value.name} cannot be null or empty", + message="CrossSectionLocation.friction_value cannot be null or empty", ) ] CHECKS += [ @@ -575,7 +575,9 @@ def is_none_or_empty(col): | (models.CrossSectionDefinition.friction_values == "") ) ), - message="Either friction value or friction values must be defined for a TABULATED YZ shape", + message=f"Either {models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + f"or {models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" + f"must be defined for a {constants.CrossSectionShape.TABULATED_YZ} cross section shape", ) ] CHECKS += [ @@ -2834,7 +2836,10 @@ def is_none_or_empty(col): != constants.CrossSectionShape.TABULATED_YZ ) .filter(col.is_not(None)), - message=(f"{col} can only be used in combination with a TABULATED_YZ"), + message=( + f"{models.CrossSectionDefinition.name}.{col.name} can only be used in combination with " + f"a {constants.CrossSectionShape.TABULATED_YZ.shape} cross section shape" + ), ) for col in veg_par_cols ] @@ -2871,7 +2876,8 @@ def is_none_or_empty(col): models.CrossSectionLocation.friction_type.is_(constants.FrictionType.CHEZY) ), message=( - f"Both {col_csloc} and {col_csdef} defined without conveyane; {col_csloc} will be used" + f"Both {col_csloc.table.name}.{col_csloc.name} and {col_csdef.table.name}.{col_csdef.name}" + f" defined without conveyance; {col_csloc.table.name}.{col_csloc.name} will be used" ), ) for col_csloc, col_csdef in [ @@ -2911,7 +2917,8 @@ def is_none_or_empty(col): ) ), message=( - f"Both {col_csloc} and {col_csdef} defined with conveyane; {col_csdef} will be used" + f"Both {col_csloc.table.name}.{col_csloc.name} and {col_csdef.table.name}.{col_csdef.name}" + f" defined without conveyance; {col_csdef.table.name}.{col_csdef.name} will be used" ), ) for col_csloc, col_csdef in [ @@ -2960,8 +2967,11 @@ def is_none_or_empty(col): & models.CrossSectionLocation.friction_value.is_not(None) ) ), - message="Both {friction_value} and {friction_values} are defined for conveyance friction. " - "Only {friction_values} will be used.", + message=f"Both {models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" + f"and {models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + f"are defined for conveyance friction. Only " + f"{models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" + f"will be used", ), QueryCheck( error_code=183, @@ -2989,8 +2999,11 @@ def is_none_or_empty(col): & models.CrossSectionLocation.friction_value.is_not(None) ) ), - message="Both {friction_value} and {friction_values} are defined for non-conveyance friction. " - "Only {friction_value} will be used.", + message=f"Both {models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" + f"and {models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + f"are defined for non-conveyance friction. Only " + f"{models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + f"will be used", ), ] CHECKS += [ @@ -3073,7 +3086,9 @@ def is_none_or_empty(col): ) ) .filter(col.is_not(None)), - message=(f"{col} cannot be used with Manning type friction"), + message=( + f"{col.table.name}.{col.name} cannot be used with Manning type friction" + ), ) for col in [ models.CrossSectionLocation.vegetation_drag_coefficient, @@ -3103,7 +3118,9 @@ def is_none_or_empty(col): & col.is_not(None) ) ), - message=(f"{col} cannot be used with Manning type friction"), + message=( + f"{col.table.name}.{col.name} cannot be used with MANNING type friction" + ), ) for col in [ models.CrossSectionDefinition.vegetation_drag_coefficients, From f51b04734d4de90c1b231051b6b51bf3f0847c4d Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Tue, 23 Jan 2024 11:46:58 +0100 Subject: [PATCH 12/22] Clean up error codes --- threedi_modelchecker/config.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 3d7c89df..de357e6c 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2817,8 +2817,6 @@ def is_none_or_empty(col): ) ] -# TODO: reconsider number because 01xx exists! -# TODO add friction value / friction_values check here ## 018x cross section parameters (continues 008x) veg_par_cols = [ models.CrossSectionDefinition.vegetation_drag_coefficients, @@ -3014,14 +3012,13 @@ def is_none_or_empty(col): for col in veg_par_cols + [models.CrossSectionDefinition.friction_values] ] -## Friction values - move - give correct number -## 9999 +## Friction values range; matches error codes for friction value checks CHECKS += [ CrossSectionVariableFrictionRangeCheck( min_value=0, max_value=1, right_inclusive=False, - error_code=9999, + error_code=22, column=models.CrossSectionDefinition.friction_values, shapes=(constants.CrossSectionShape.TABULATED_YZ,), friction_types=[ @@ -3033,7 +3030,7 @@ def is_none_or_empty(col): CHECKS += [ CrossSectionVariableFrictionRangeCheck( min_value=0, - error_code=9999, + error_code=21, column=models.CrossSectionDefinition.friction_values, shapes=(constants.CrossSectionShape.TABULATED_YZ,), friction_types=[ From f99393e510d8205ee180fb8dce5fec70baf56761 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Tue, 23 Jan 2024 11:55:13 +0100 Subject: [PATCH 13/22] Upgrade schema version and fix some bugs --- pyproject.toml | 2 +- threedi_modelchecker/config.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e2b10e0c..9e7fa63c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ dependencies = [ "Click", "GeoAlchemy2>=0.9,!=0.11.*", "SQLAlchemy>=1.4", - "threedi-schema==0.218.*", + "threedi-schema==0.219.*", ] [project.optional-dependencies] diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index de357e6c..93f864eb 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -575,8 +575,8 @@ def is_none_or_empty(col): | (models.CrossSectionDefinition.friction_values == "") ) ), - message=f"Either {models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" - f"or {models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" + message=f"Either {models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" + f"or {models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" f"must be defined for a {constants.CrossSectionShape.TABULATED_YZ} cross section shape", ) ] @@ -2835,8 +2835,8 @@ def is_none_or_empty(col): ) .filter(col.is_not(None)), message=( - f"{models.CrossSectionDefinition.name}.{col.name} can only be used in combination with " - f"a {constants.CrossSectionShape.TABULATED_YZ.shape} cross section shape" + f"{col.table.name}.{col.name} can only be used in combination with " + f"a {constants.CrossSectionShape.TABULATED_YZ.name} cross section shape" ), ) for col in veg_par_cols @@ -2965,10 +2965,10 @@ def is_none_or_empty(col): & models.CrossSectionLocation.friction_value.is_not(None) ) ), - message=f"Both {models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" - f"and {models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + message=f"Both {models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" + f"and {models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" f"are defined for conveyance friction. Only " - f"{models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" + f"{models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" f"will be used", ), QueryCheck( @@ -2997,10 +2997,10 @@ def is_none_or_empty(col): & models.CrossSectionLocation.friction_value.is_not(None) ) ), - message=f"Both {models.CrossSectionDefinition.name}.{models.CrossSectionDefinition.friction_values.name}" - f"and {models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + message=f"Both {models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" + f"and {models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" f"are defined for non-conveyance friction. Only " - f"{models.CrossSectionLocation.name}.{models.CrossSectionLocation.friction_value.name}" + f"{models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" f"will be used", ), ] From 90a7f13bda78dd1f3f0a3a8e2487216980620bd9 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Wed, 24 Jan 2024 10:49:14 +0100 Subject: [PATCH 14/22] Include friction values in check --- threedi_modelchecker/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 93f864eb..74ccbaf6 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2839,7 +2839,7 @@ def is_none_or_empty(col): f"a {constants.CrossSectionShape.TABULATED_YZ.name} cross section shape" ), ) - for col in veg_par_cols + for col in veg_par_cols + models.CrossSectionDefinition.friction_values ] CHECKS += [ CrossSectionVariableCorrectLengthCheck( From 6c125d66f73249bdaaacb09426ec0876807b6f5d Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Thu, 25 Jan 2024 12:11:41 +0100 Subject: [PATCH 15/22] Fix bug in combining list and item --- threedi_modelchecker/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 74ccbaf6..5a50abb0 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2839,7 +2839,7 @@ def is_none_or_empty(col): f"a {constants.CrossSectionShape.TABULATED_YZ.name} cross section shape" ), ) - for col in veg_par_cols + models.CrossSectionDefinition.friction_values + for col in veg_par_cols + [models.CrossSectionDefinition.friction_values] ] CHECKS += [ CrossSectionVariableCorrectLengthCheck( From 1869f68ae376f195aa96530e7b660bb4b5d2c04a Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 29 Jan 2024 09:26:06 +0100 Subject: [PATCH 16/22] Clarify message and parameter names --- .../checks/cross_section_definitions.py | 2 +- threedi_modelchecker/config.py | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index a34d37df..acf4a3bc 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -606,7 +606,7 @@ def get_invalid(self, session): return invalids def description(self): - return f"{self.column_name} should contain exactly 1 value less as the lenght and width" + return f"{self.column_name} should contain 1 value for each element; len({self.column_name}) = len(width)-1" class CrossSectionVariableRangeCheck(CrossSectionBaseCheck): diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 5a50abb0..6c9d01a8 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2818,7 +2818,7 @@ def is_none_or_empty(col): ] ## 018x cross section parameters (continues 008x) -veg_par_cols = [ +vegetation_parameter_columns = [ models.CrossSectionDefinition.vegetation_drag_coefficients, models.CrossSectionDefinition.vegetation_heights, models.CrossSectionDefinition.vegetation_stem_diameters, @@ -2839,7 +2839,8 @@ def is_none_or_empty(col): f"a {constants.CrossSectionShape.TABULATED_YZ.name} cross section shape" ), ) - for col in veg_par_cols + [models.CrossSectionDefinition.friction_values] + for col in vegetation_parameter_columns + + [models.CrossSectionDefinition.friction_values] ] CHECKS += [ CrossSectionVariableCorrectLengthCheck( @@ -2848,7 +2849,7 @@ def is_none_or_empty(col): shapes=(constants.CrossSectionShape.TABULATED_YZ,), filters=models.CrossSectionDefinition.height.is_not(None) & col.is_not(None), ) - for col in veg_par_cols + for col in vegetation_parameter_columns ] CHECKS += [ CrossSectionFloatListCheck( @@ -2856,7 +2857,7 @@ def is_none_or_empty(col): column=col, shapes=(constants.CrossSectionShape.TABULATED_YZ,), ) - for col in veg_par_cols + for col in vegetation_parameter_columns ] CHECKS += [ QueryCheck( @@ -2901,25 +2902,28 @@ def is_none_or_empty(col): QueryCheck( error_code=182, level=CheckLevel.WARNING, - column=col_csloc, + column=col_cross_section_location, invalid=Query(models.CrossSectionDefinition) .join( models.CrossSectionLocation, models.CrossSectionLocation.definition_id == models.CrossSectionDefinition.id, ) - .filter(col_csloc.is_not(None) & col_csdef.is_not(None)) + .filter( + col_cross_section_location.is_not(None) + & col_cross_section_definition.is_not(None) + ) .filter( models.CrossSectionLocation.friction_type.is_( constants.FrictionType.CHEZY_CONVEYANCE ) ), message=( - f"Both {col_csloc.table.name}.{col_csloc.name} and {col_csdef.table.name}.{col_csdef.name}" - f" defined without conveyance; {col_csdef.table.name}.{col_csdef.name} will be used" + f"Both {col_cross_section_location.table.name}.{col_cross_section_location.name} and {col_cross_section_definition.table.name}.{col_cross_section_definition.name}" + f" defined without conveyance; {col_cross_section_definition.table.name}.{col_cross_section_definition.name} will be used" ), ) - for col_csloc, col_csdef in [ + for col_cross_section_location, col_cross_section_definition in [ ( models.CrossSectionLocation.vegetation_drag_coefficient, models.CrossSectionDefinition.vegetation_drag_coefficients, @@ -3009,7 +3013,8 @@ def is_none_or_empty(col): error_code=184, column=col, ) - for col in veg_par_cols + [models.CrossSectionDefinition.friction_values] + for col in vegetation_parameter_columns + + [models.CrossSectionDefinition.friction_values] ] ## Friction values range; matches error codes for friction value checks From 0fe8960f36e7ad3fb5f1437269dda3dba380279d Mon Sep 17 00:00:00 2001 From: margrietpalm Date: Mon, 29 Jan 2024 09:27:56 +0100 Subject: [PATCH 17/22] Fix typo in message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eli Sallé --- threedi_modelchecker/checks/cross_section_definitions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/threedi_modelchecker/checks/cross_section_definitions.py b/threedi_modelchecker/checks/cross_section_definitions.py index acf4a3bc..766825e5 100644 --- a/threedi_modelchecker/checks/cross_section_definitions.py +++ b/threedi_modelchecker/checks/cross_section_definitions.py @@ -586,7 +586,7 @@ def description(self): class CrossSectionVariableCorrectLengthCheck(CrossSectionBaseCheck): - """Variable friction and vegetation properties should 1 value for each element; len(var_property) = len(width)-1""" + """Variable friction and vegetation properties should contain 1 value for each element; len(var_property) = len(width)-1""" def get_invalid(self, session): invalids = [] From 47b95f12d7990dd0a3c40ad54f9aaa0a1ad36819 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 29 Jan 2024 09:49:54 +0100 Subject: [PATCH 18/22] Rename some more variables --- threedi_modelchecker/config.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 6c9d01a8..d5df73bf 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -2863,23 +2863,26 @@ def is_none_or_empty(col): QueryCheck( error_code=182, level=CheckLevel.WARNING, - column=col_csloc, + column=col_cross_section_location, invalid=Query(models.CrossSectionDefinition) .join( models.CrossSectionLocation, models.CrossSectionLocation.definition_id == models.CrossSectionDefinition.id, ) - .filter(col_csloc.is_not(None) & col_csdef.is_not(None)) + .filter( + col_cross_section_location.is_not(None) + & col_cross_section_definition.is_not(None) + ) .filter( models.CrossSectionLocation.friction_type.is_(constants.FrictionType.CHEZY) ), message=( - f"Both {col_csloc.table.name}.{col_csloc.name} and {col_csdef.table.name}.{col_csdef.name}" - f" defined without conveyance; {col_csloc.table.name}.{col_csloc.name} will be used" + f"Both {col_cross_section_location.table.name}.{col_cross_section_location.name} and {col_cross_section_definition.table.name}.{col_cross_section_definition.name}" + f" defined without conveyance; {col_cross_section_location.table.name}.{col_cross_section_location.name} will be used" ), ) - for col_csloc, col_csdef in [ + for col_cross_section_location, col_cross_section_definition in [ ( models.CrossSectionLocation.vegetation_drag_coefficient, models.CrossSectionDefinition.vegetation_drag_coefficients, From 254272eac69cce1ecd9d6600da2d147e6e5e6dfa Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 29 Jan 2024 15:27:39 +0100 Subject: [PATCH 19/22] Remove duplicate check --- threedi_modelchecker/config.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index d5df73bf..4cb20b6c 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -706,16 +706,9 @@ def is_none_or_empty(col): CHECKS += [ CrossSectionFloatListCheck( error_code=87, - column=col, + column=models.CrossSectionDefinition.friction_values, shapes=(constants.CrossSectionShape.TABULATED_YZ,), ) - for col in [ - models.CrossSectionDefinition.friction_values, - models.CrossSectionDefinition.vegetation_drag_coefficients, - models.CrossSectionDefinition.vegetation_heights, - models.CrossSectionDefinition.vegetation_stem_diameters, - models.CrossSectionDefinition.vegetation_stem_densities, - ] ] ## 01xx: LEVEL CHECKS @@ -2859,6 +2852,15 @@ def is_none_or_empty(col): ) for col in vegetation_parameter_columns ] +CHECKS += [ + CrossSectionFloatListCheck( + error_code=87, + column=col, + shapes=(constants.CrossSectionShape.TABULATED_YZ,), + ) + for col in vegetation_parameter_columns +] + CHECKS += [ QueryCheck( error_code=182, From 439f5bde524c42850c38832a51129eab4f05f54f Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 29 Jan 2024 15:27:56 +0100 Subject: [PATCH 20/22] Update changelog --- CHANGES.rst | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 8dee3b87..4e377df9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,18 @@ Changelog of threedi-modelchecker 2.5.3 (unreleased) ------------------ -- Nothing changed yet. +- Add error check (0003) for CrossSectionLocation.friction_value because that check is no longer included in the factory checks. +- Add error checks (0021, 0022) for friction value ranges +- Add error check (0080) for absent CrossSectionLocation.friction_value and CrossSectionDefintion.friction_values for TABULATED_YZ shape +- Add error check (0087) for correct formatting of space separated list of values for variable friction +- Add error check (0180) for variable friction and variable vegetation parameters only be used together with TABULATED_YZ shape +- Add error check (0181) for correct number of values for variable friction and variable vegetation parameters +- Add warning check (0182) for cases where fixed and variable vegetation parameters are used +- Add warning check (0183) for cases where fixed and variable friction value(s) are used +- Add error check (0184) for using variable friction or vegetation with open, monotonically increasing z profile +- Add error check (0187) for correct formatting of space separated list of variable vegetation parameters +- Add error check (0190) for non-negative vegetation parameters +- Add error check (0191) for disallowing vegetation with Manning friction 2.5.2 (2024-01-19) From dee64078d4f5faf0daf9b370e3490cdf6fee345f Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Mon, 29 Jan 2024 16:24:09 +0100 Subject: [PATCH 21/22] Renumber and reorder some checks, remove duplicates, update changelog --- CHANGES.rst | 21 +++++++++------ threedi_modelchecker/config.py | 47 ++++++++++++++-------------------- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 4e377df9..43c10d72 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,18 +5,23 @@ Changelog of threedi-modelchecker 2.5.3 (unreleased) ------------------ -- Add error check (0003) for CrossSectionLocation.friction_value because that check is no longer included in the factory checks. -- Add error checks (0021, 0022) for friction value ranges -- Add error check (0080) for absent CrossSectionLocation.friction_value and CrossSectionDefintion.friction_values for TABULATED_YZ shape +- Add error check (0020) for CrossSectionLocation.friction_value because that check is no longer included in the factory checks. +- Add error check (0080) for absent CrossSectionLocation.friction_value and CrossSectionDefinition.friction_values for TABULATED_YZ shape - Add error check (0087) for correct formatting of space separated list of values for variable friction - Add error check (0180) for variable friction and variable vegetation parameters only be used together with TABULATED_YZ shape - Add error check (0181) for correct number of values for variable friction and variable vegetation parameters -- Add warning check (0182) for cases where fixed and variable vegetation parameters are used -- Add warning check (0183) for cases where fixed and variable friction value(s) are used -- Add error check (0184) for using variable friction or vegetation with open, monotonically increasing z profile +- Add warning check (0182) for fixed and variable vegetation parameters in combination with non-conveyance friction +- Add warning check (0183) for fixed and variable vegetation parameters in combination with conveyance friction +- Add warning check (0184) for fixed and variable friction in combination with non-conveyance friction +- Add warning check (0185) for fixed and variable friction in combination with conveyance friction +- Add error check (0186) for using variable friction or vegetation with open, monotonically increasing z profile - Add error check (0187) for correct formatting of space separated list of variable vegetation parameters -- Add error check (0190) for non-negative vegetation parameters -- Add error check (0191) for disallowing vegetation with Manning friction +- Add error check (0188) for all friction values non-negative and smaller than 1 for Manning friction +- Add error check (0189) for all friction values non-negative for Chezy friction +- Add error check (0190) for non-negative fixed vegetation parameters +- Add error check (0191) for non-negative variable vegetation parameters +- Add error check (0192) for disallowing fixed vegetation with Manning friction +- Add error check (0193) for disallowing variable vegetation with Manning friction 2.5.2 (2024-01-19) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 4cb20b6c..64deb475 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -172,7 +172,7 @@ def is_none_or_empty(col): ## Use same error code as other null checks CHECKS += [ QueryCheck( - error_code=3, + error_code=20, column=models.CrossSectionLocation.friction_value, invalid=( Query(models.CrossSectionLocation) @@ -2852,15 +2852,6 @@ def is_none_or_empty(col): ) for col in vegetation_parameter_columns ] -CHECKS += [ - CrossSectionFloatListCheck( - error_code=87, - column=col, - shapes=(constants.CrossSectionShape.TABULATED_YZ,), - ) - for col in vegetation_parameter_columns -] - CHECKS += [ QueryCheck( error_code=182, @@ -2905,7 +2896,7 @@ def is_none_or_empty(col): ] CHECKS += [ QueryCheck( - error_code=182, + error_code=183, level=CheckLevel.WARNING, column=col_cross_section_location, invalid=Query(models.CrossSectionDefinition) @@ -2949,7 +2940,7 @@ def is_none_or_empty(col): ] CHECKS += [ QueryCheck( - error_code=183, + error_code=184, level=CheckLevel.WARNING, column=models.CrossSectionDefinition.friction_values, invalid=( @@ -2962,11 +2953,11 @@ def is_none_or_empty(col): .filter( ( models.CrossSectionLocation.friction_type - == constants.FrictionType.CHEZY_CONVEYANCE + == constants.FrictionType.CHEZY ) | ( models.CrossSectionLocation.friction_type - == constants.FrictionType.MANNING_CONVEYANCE + == constants.FrictionType.MANNING ) ) .filter( @@ -2976,12 +2967,12 @@ def is_none_or_empty(col): ), message=f"Both {models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" f"and {models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" - f"are defined for conveyance friction. Only " - f"{models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" + f"are defined for non-conveyance friction. Only " + f"{models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" f"will be used", ), QueryCheck( - error_code=183, + error_code=185, level=CheckLevel.WARNING, column=models.CrossSectionDefinition.friction_values, invalid=( @@ -2994,11 +2985,11 @@ def is_none_or_empty(col): .filter( ( models.CrossSectionLocation.friction_type - == constants.FrictionType.CHEZY + == constants.FrictionType.CHEZY_CONVEYANCE ) | ( models.CrossSectionLocation.friction_type - == constants.FrictionType.MANNING + == constants.FrictionType.MANNING_CONVEYANCE ) ) .filter( @@ -3008,27 +2999,27 @@ def is_none_or_empty(col): ), message=f"Both {models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" f"and {models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" - f"are defined for non-conveyance friction. Only " - f"{models.CrossSectionLocation.friction_value.table.name}.{models.CrossSectionLocation.friction_value.name}" + f"are defined for conveyance friction. Only " + f"{models.CrossSectionDefinition.friction_values.table.name}.{models.CrossSectionDefinition.friction_values.name}" f"will be used", ), ] CHECKS += [ OpenIncreasingCrossSectionVariableCheck( - error_code=184, + error_code=186, column=col, ) for col in vegetation_parameter_columns + [models.CrossSectionDefinition.friction_values] ] -## Friction values range; matches error codes for friction value checks +## Friction values range CHECKS += [ CrossSectionVariableFrictionRangeCheck( min_value=0, max_value=1, right_inclusive=False, - error_code=22, + error_code=188, column=models.CrossSectionDefinition.friction_values, shapes=(constants.CrossSectionShape.TABULATED_YZ,), friction_types=[ @@ -3040,7 +3031,7 @@ def is_none_or_empty(col): CHECKS += [ CrossSectionVariableFrictionRangeCheck( min_value=0, - error_code=21, + error_code=189, column=models.CrossSectionDefinition.friction_values, shapes=(constants.CrossSectionShape.TABULATED_YZ,), friction_types=[ @@ -3066,7 +3057,7 @@ def is_none_or_empty(col): ] CHECKS += [ CrossSectionVariableRangeCheck( - error_code=190, + error_code=191, min_value=0, column=col, shapes=(constants.CrossSectionShape.TABULATED_YZ,), @@ -3081,7 +3072,7 @@ def is_none_or_empty(col): CHECKS += [ QueryCheck( - error_code=191, + error_code=192, column=col, invalid=Query(models.CrossSectionLocation) .filter( @@ -3106,7 +3097,7 @@ def is_none_or_empty(col): ] CHECKS += [ QueryCheck( - error_code=191, + error_code=193, column=col, invalid=( Query(models.CrossSectionDefinition) From 01b07aef7ec6d44b952c615d4fc09ae146a04405 Mon Sep 17 00:00:00 2001 From: Margriet Palm Date: Wed, 31 Jan 2024 12:10:44 +0100 Subject: [PATCH 22/22] Add check that ensures that either none or all vegetation parameters are defined --- CHANGES.rst | 3 + threedi_modelchecker/checks/other.py | 105 ++++++++++++- threedi_modelchecker/config.py | 53 ++++--- .../tests/test_checks_other.py | 148 ++++++++++++++++++ 4 files changed, 283 insertions(+), 26 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 43c10d72..1a575d81 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -22,6 +22,9 @@ Changelog of threedi-modelchecker - Add error check (0191) for non-negative variable vegetation parameters - Add error check (0192) for disallowing fixed vegetation with Manning friction - Add error check (0193) for disallowing variable vegetation with Manning friction +- Add error check (0194) for requiring that either all or none fixed vegetation parameters are defined +- Add error check (0195) for requiring that either all or none variable vegetation parameters are defined + 2.5.2 (2024-01-19) diff --git a/threedi_modelchecker/checks/other.py b/threedi_modelchecker/checks/other.py index d714beac..a8c7454a 100644 --- a/threedi_modelchecker/checks/other.py +++ b/threedi_modelchecker/checks/other.py @@ -1,9 +1,10 @@ +from abc import ABC, abstractmethod from dataclasses import dataclass from typing import List, Literal, NamedTuple -from sqlalchemy import case, cast, distinct, func, REAL, select, text +from sqlalchemy import and_, case, cast, distinct, func, not_, REAL, select, text from sqlalchemy.orm import aliased, Query, Session -from threedi_schema import constants, models +from threedi_schema.domain import constants, models from .base import BaseCheck, CheckLevel from .cross_section_definitions import cross_section_configuration @@ -959,3 +960,103 @@ def get_invalid(self, session: Session) -> List[NamedTuple]: def description(self) -> str: return f"The value you have used for {self.column_name} is still in beta; please do not use it yet." + + +class AllPresent(BaseCheck, ABC): + """Base class to check if all or none values are present for a list of columns""" + + def __init__(self, columns, *args, **kwargs): + self.columns = columns + super().__init__(*args, **kwargs) + + @abstractmethod + def _get_records(self, session): + pass + + def get_invalid(self, session): + # Create filters that find all rows where all or none of the values are present + filter_condition_all = and_( + *[(col != None) & (col != "") for col in self.columns] + ) + filter_condition_none = and_( + *[(col == None) | (col == "") for col in self.columns] + ) + # Return all rows where neither all or none values are present + return ( + self._get_records(session) + .filter(not_(filter_condition_all)) + .filter(not_(filter_condition_none)) + .all() + ) + + def description(self): + column_string = ",".join( + [f"{column.table.name}.{column.name}" for column in self.columns] + ) + return f"All of these columns must be defined: {column_string}" + + +class AllPresentFixedVegetationParameters(AllPresent): + """Check if all or none vegetation values are defined in the CrossSectionLocation table""" + + def __init__(self, *args, **kwargs): + columns = [ + models.CrossSectionLocation.vegetation_drag_coefficient, + models.CrossSectionLocation.vegetation_height, + models.CrossSectionLocation.vegetation_stem_diameter, + models.CrossSectionLocation.vegetation_stem_density, + ] + super().__init__(columns, *args, **kwargs) + + def _get_records(self, session): + # Get records with valid settings for vegetation in CrossSectionLocation + return ( + session.query(models.CrossSectionLocation) + .join( + models.CrossSectionDefinition, + models.CrossSectionDefinition.id + == models.CrossSectionLocation.definition_id, + ) + .filter( + models.CrossSectionLocation.friction_type.is_( + constants.FrictionType.CHEZY + ) + ) + .filter( + models.CrossSectionDefinition.shape.is_( + constants.CrossSectionShape.TABULATED_YZ + ) + ) + ) + + +class AllPresentVariableVegetationParameters(AllPresent): + def __init__(self, *args, **kwargs): + columns = [ + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, + ] + super().__init__(columns, *args, **kwargs) + + def _get_records(self, session): + # Get records with valid settings for vegetation in CrossSectionDefinition + return ( + session.query(models.CrossSectionDefinition) + .join( + models.CrossSectionLocation, + models.CrossSectionDefinition.id + == models.CrossSectionLocation.definition_id, + ) + .filter( + models.CrossSectionLocation.friction_type.is_( + constants.FrictionType.CHEZY_CONVEYANCE + ) + ) + .filter( + models.CrossSectionDefinition.shape.is_( + constants.CrossSectionShape.TABULATED_YZ + ) + ) + ) diff --git a/threedi_modelchecker/config.py b/threedi_modelchecker/config.py index 64deb475..0c5258be 100644 --- a/threedi_modelchecker/config.py +++ b/threedi_modelchecker/config.py @@ -47,6 +47,8 @@ generate_unique_checks, ) from .checks.other import ( + AllPresentFixedVegetationParameters, + AllPresentVariableVegetationParameters, BetaColumnsCheck, BetaValuesCheck, BoundaryCondition1DObjectNumberCheck, @@ -3042,18 +3044,26 @@ def is_none_or_empty(col): ] ## 019x vegetation parameter checks +vegetation_parameter_columns_singular = [ + models.CrossSectionLocation.vegetation_drag_coefficient, + models.CrossSectionLocation.vegetation_height, + models.CrossSectionLocation.vegetation_stem_diameter, + models.CrossSectionLocation.vegetation_stem_density, +] +vegetation_parameter_columns_plural = [ + models.CrossSectionDefinition.vegetation_drag_coefficients, + models.CrossSectionDefinition.vegetation_heights, + models.CrossSectionDefinition.vegetation_stem_diameters, + models.CrossSectionDefinition.vegetation_stem_densities, +] + CHECKS += [ RangeCheck( error_code=190, column=col, min_value=0, ) - for col in [ - models.CrossSectionLocation.vegetation_drag_coefficient, - models.CrossSectionLocation.vegetation_height, - models.CrossSectionLocation.vegetation_stem_diameter, - models.CrossSectionLocation.vegetation_stem_density, - ] + for col in vegetation_parameter_columns_singular ] CHECKS += [ CrossSectionVariableRangeCheck( @@ -3062,12 +3072,7 @@ def is_none_or_empty(col): column=col, shapes=(constants.CrossSectionShape.TABULATED_YZ,), ) - for col in [ - models.CrossSectionDefinition.vegetation_drag_coefficients, - models.CrossSectionDefinition.vegetation_heights, - models.CrossSectionDefinition.vegetation_stem_diameters, - models.CrossSectionDefinition.vegetation_stem_densities, - ] + for col in vegetation_parameter_columns_plural ] CHECKS += [ @@ -3088,12 +3093,7 @@ def is_none_or_empty(col): f"{col.table.name}.{col.name} cannot be used with Manning type friction" ), ) - for col in [ - models.CrossSectionLocation.vegetation_drag_coefficient, - models.CrossSectionLocation.vegetation_height, - models.CrossSectionLocation.vegetation_stem_diameter, - models.CrossSectionLocation.vegetation_stem_density, - ] + for col in vegetation_parameter_columns_singular ] CHECKS += [ QueryCheck( @@ -3120,12 +3120,17 @@ def is_none_or_empty(col): f"{col.table.name}.{col.name} cannot be used with MANNING type friction" ), ) - for col in [ - models.CrossSectionDefinition.vegetation_drag_coefficients, - models.CrossSectionDefinition.vegetation_heights, - models.CrossSectionDefinition.vegetation_stem_diameters, - models.CrossSectionDefinition.vegetation_stem_densities, - ] + for col in vegetation_parameter_columns_plural +] +CHECKS += [ + AllPresentFixedVegetationParameters( + error_code=194, + column=vegetation_parameter_columns_singular[0], + ), + AllPresentVariableVegetationParameters( + error_code=195, + column=vegetation_parameter_columns_plural[0], + ), ] # These checks are optional, depending on a command line argument diff --git a/threedi_modelchecker/tests/test_checks_other.py b/threedi_modelchecker/tests/test_checks_other.py index 7b8888d8..03d368c1 100644 --- a/threedi_modelchecker/tests/test_checks_other.py +++ b/threedi_modelchecker/tests/test_checks_other.py @@ -7,6 +7,8 @@ from threedi_schema.beta_features import BETA_COLUMNS, BETA_VALUES from threedi_modelchecker.checks.other import ( + AllPresentFixedVegetationParameters, + AllPresentVariableVegetationParameters, BetaColumnsCheck, BetaValuesCheck, ChannelManholeLevelCheck, @@ -696,3 +698,149 @@ def test_beta_features_in_server(threedi_db, allow_beta_features, no_checks_expe assert len(model_beta_checks) == 0 else: assert len(model_beta_checks) > 0 + + +@pytest.mark.parametrize( + "cols, shape, friction_type, result", + [ + # single column defined: should fail + ( + ["vegetation_height"], + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY, + False, + ), + # both columns defined, but one empty: should fail + ( + ["vegetation_height", "vegetation_stem_diameter"], + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY, + False, + ), + # no columns defined: should pass + ( + [], + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY, + True, + ), + # both columns defined: should pass + ( + [ + "vegetation_drag_coefficient", + "vegetation_height", + "vegetation_stem_diameter", + "vegetation_stem_density", + ], + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY, + True, + ), + # shape is not included in check: should pass + ( + ["vegetation_height"], + constants.CrossSectionShape.RECTANGLE, + constants.FrictionType.CHEZY, + True, + ), + # friction type in not included in check: should pass + ( + ["vegetation_height"], + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.MANNING, + True, + ), + ], +) +def test_all_present_fixed_vegetation_parameters( + session, cols, shape, friction_type, result +): + definition = factories.CrossSectionDefinitionFactory( + shape=shape, + friction_values="1", + ) + veg_args = {col: 1 for col in cols} + factories.CrossSectionLocationFactory( + definition=definition, friction_type=friction_type, **veg_args + ) + check = AllPresentFixedVegetationParameters( + column=models.CrossSectionLocation.vegetation_height + ) + invalid_rows = check.get_invalid(session) + assert (len(invalid_rows) == 0) == result + + +# TODO: add check for Variable... +@pytest.mark.parametrize( + "cols, val, shape, friction_type, result", + [ + # single column defined: should fail + ( + ["vegetation_heights"], + "1 2", + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY_CONVEYANCE, + False, + ), + # both columns defined, but one empty: should fail + ( + ["vegetation_heights", "vegetation_stem_diameters"], + "1 2", + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY_CONVEYANCE, + False, + ), + # no columns defined: should pass + ( + [], + "1 2", + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY_CONVEYANCE, + True, + ), + # both columns defined: should pass + ( + [ + "vegetation_drag_coefficients", + "vegetation_heights", + "vegetation_stem_diameters", + "vegetation_stem_densities", + ], + "1 2", + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.CHEZY_CONVEYANCE, + True, + ), + # shape is not included in check: should pass + ( + ["vegetation_heights"], + "1 2", + constants.CrossSectionShape.RECTANGLE, + constants.FrictionType.CHEZY_CONVEYANCE, + True, + ), + # friction type in not included in check: should pass + ( + ["vegetation_heights"], + "1 2", + constants.CrossSectionShape.TABULATED_YZ, + constants.FrictionType.MANNING, + True, + ), + ], +) +def test_all_present_variable_vegetation_parameters( + session, cols, val, shape, friction_type, result +): + veg_args = {col: val for col in cols} + definition = factories.CrossSectionDefinitionFactory( + shape=shape, friction_values="1 2", **veg_args + ) + factories.CrossSectionLocationFactory( + definition=definition, friction_type=friction_type + ) + check = AllPresentVariableVegetationParameters( + column=models.CrossSectionDefinition.vegetation_heights + ) + invalid_rows = check.get_invalid(session) + assert (len(invalid_rows) == 0) == result