293 add support for beta features in spatialite models (#294)

CHANGES.rst

@@ -5,6 +5,11 @@ Changelog of threedi-modelchecker
 2.0.2 (unreleased)
 ------------------
 
+- Add support for designating beta features in threedi-schema. If a user puts a
+  non-null value in a column marked as beta in threedi-schema, a BetaFeaturesCheck
+  error 1300 will be raised by the modelchecker. The allow-beta flag has been added
+  to the CLI interface to disable this check temporarily.
+
 - Add errors and warnings for vegetation_drag input. Both rasters and global values.
 
 - Added check 73: groundwater boundaries are allowed only when there is
@@ -20,7 +25,7 @@ Changelog of threedi-modelchecker
 2.0.1 (2023-03-20)
 ------------------
 
-- Pin minor version for threede-schema dependency.
+- Pin minor version for threedi-schema dependency.
 
 
 2.0.0 (2023-03-20)

README.rst

@@ -52,9 +52,10 @@ Command-line interface
 
 Use the modelchecker from the command line as follows::
 
-    threedi_modelchecker -s path/to/model.sqlite check -l warning 
+    threedi_modelchecker check -s path/to/model.sqlite -l warning 
 
-By default, WARNING and INFO checks are ignored.
+By default, WARNING and INFO checks are ignored. To skip the beta features check,
+add the --allow-beta flag.
 
 
 Development
@@ -71,7 +72,7 @@ Run the tests:
 
     docker-compose run modelchecker pytest
 
-See `Creating revisions <threedi_modelchecker/migrations/README.rst>`_ for 
+See `Creating revisions <https://github.com/nens/threedi-schema/blob/master/threedi_schema/migrations/README.rst>`_ for 
 instructions on how to change the 3Di model schematisation.
 
 Release

setup.py

@@ -5,7 +5,7 @@
 long_description = "\n\n".join([open("README.rst").read()])
 
 install_requires = [
-    "threedi-schema==0.216.*",
+    "threedi-schema==0.216.*,>=0.216.2",
     "Click",
     "GeoAlchemy2>=0.9,!=0.11.*",
     "SQLAlchemy>=1.4",

threedi_modelchecker/checks/other.py

@@ -551,3 +551,34 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
 
     def description(self) -> str:
         return f"{self.column_name} will empty its storage faster than one timestep, which can cause simulation instabilities"
+
+
+class BetaColumnsCheck(BaseCheck):
+    """Check that no beta columns were used in the database"""
+
+    def get_invalid(self, session: Session) -> List[NamedTuple]:
+        return session.query(self.table).filter(self.column.isnot(None)).all()
+
+    def description(self) -> str:
+        return f"{self.column_name} is a beta feature, which is still under development; please do not use it yet."
+
+
+class BetaValuesCheck(BaseCheck):
+    """Check that no beta features were used in the database"""
+
+    def __init__(
+        self,
+        column,
+        values: list = [],
+        filters=None,
+        level=CheckLevel.ERROR,
+        error_code=0,
+    ):
+        super().__init__(column, filters, level, error_code)
+        self.values = values
+
+    def get_invalid(self, session: Session) -> List[NamedTuple]:
+        return session.query(self.table).filter(self.column.in_(self.values)).all()
+
+    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."

threedi_modelchecker/config.py

@@ -3,6 +3,7 @@
 from sqlalchemy import func
 from sqlalchemy.orm import Query
 from threedi_schema import constants, models
+from threedi_schema.beta_features import BETA_COLUMNS, BETA_VALUES
 
 from .checks import geo_query
 from .checks.base import (
@@ -39,6 +40,8 @@
     generate_unique_checks,
 )
 from .checks.other import (
+    BetaColumnsCheck,
+    BetaValuesCheck,
     BoundaryCondition1DObjectNumberCheck,
     ChannelManholeLevelCheck,
     ConnectionNodesDistance,
@@ -2463,16 +2466,38 @@ def is_none_or_empty(col):
     for control_table in (models.ControlMemory, models.ControlTable)
 ]
 
+# These checks are optional, depending on a command line argument
+beta_features_check = []
+beta_features_check += [
+    BetaColumnsCheck(
+        error_code=1300,
+        column=col,
+        level=CheckLevel.ERROR,
+    )
+    for col in BETA_COLUMNS
+]
+for pair in BETA_VALUES:
+    beta_features_check += [
+        BetaValuesCheck(
+            error_code=1300,
+            column=col,
+            values=pair["values"],
+            level=CheckLevel.ERROR,
+        )
+        for col in pair["columns"]
+    ]
+
 
 class Config:
     """Collection of checks
 
     Some checks are generated by a factory. These are usually very generic
     checks which apply to many columns, such as foreign keys."""
 
-    def __init__(self, models):
+    def __init__(self, models, allow_beta_features=False):
         self.models = models
         self.checks = []
+        self.allow_beta_features = allow_beta_features
         self.generate_checks()
 
     def generate_checks(self):
@@ -2512,6 +2537,8 @@ def generate_checks(self):
             )
 
         self.checks += CHECKS
+        if not self.allow_beta_features:
+            self.checks += beta_features_check
 
     def iter_checks(self, level=CheckLevel.ERROR):
         """Iterate over checks with at least 'level'"""

threedi_modelchecker/model_checks.py

@@ -10,7 +10,12 @@
 
 
 class ThreediModelChecker:
-    def __init__(self, threedi_db: ThreediDatabase, context: Optional[Dict] = None):
+    def __init__(
+        self,
+        threedi_db: ThreediDatabase,
+        context: Optional[Dict] = None,
+        allow_beta_features=False,
+    ):
         """Initialize the model checker.
 
         Optionally, supply the context of the model check:
@@ -23,7 +28,9 @@ def __init__(self, threedi_db: ThreediDatabase, context: Optional[Dict] = None):
         self.db = threedi_db
         self.schema = self.db.schema
         self.schema.validate_schema()
-        self.config = Config(self.models)
+        self.config = Config(
+            models=self.models, allow_beta_features=allow_beta_features
+        )
         context = {} if context is None else context.copy()
         context_type = context.pop("context_type", "local")
         if context_type == "local":

threedi_modelchecker/scripts.py

@@ -29,7 +29,13 @@ def cli():
     help="Path to an sqlite (spatialite) file",
     required=True,
 )
-def check(sqlite, file, level):
+@click.option(
+    "--allow-beta",
+    is_flag=True,
+    default=False,
+    help="Don't check whether beta features were used in the database.",
+)
+def check(sqlite, file, level, allow_beta):
     """Checks the threedi-model for errors / warnings / info messages"""
     db = ThreediDatabase(sqlite, echo=False)
     """Checks the threedi model schematisation for errors."""
@@ -44,7 +50,7 @@ def check(sqlite, file, level):
     if file:
         click.echo("Model errors will be written to %s" % file)
 
-    mc = ThreediModelChecker(db)
+    mc = ThreediModelChecker(threedi_db=db, allow_beta_features=allow_beta)
     model_errors = mc.errors(level=level)
 
     if file:

threedi_modelchecker/tests/factories.py

@@ -338,3 +338,22 @@ class Meta:
     code = "code"
     the_geom = "SRID=4326;LINESTRING(-71.06452 42.2874, -71.06452 42.286)"
     channel = factory.SubFactory(ChannelFactory)
+
+
+class VegetationDragFactory(factory.alchemy.SQLAlchemyModelFactory):
+    class Meta:
+        model = models.VegetationDrag
+        sqlalchemy_session = None
+
+    display_name = Faker("name")
+    height = 1.0
+    height_file = "height_file.txt"
+
+    stem_count = 50000
+    stem_count_file = "stem_count_file.txt"
+
+    stem_diameter = 0.5
+    stem_diameter_file = "stem_diameter_file.txt"
+
+    drag_coefficient = 0.4
+    drag_coefficient_file = "drag_coefficient_file.txt"

threedi_modelchecker/tests/test_checks_other.py

@@ -1,9 +1,14 @@
+from unittest import mock
+
 import pytest
 from sqlalchemy import func, text
 from sqlalchemy.orm import aliased, Query
-from threedi_schema import constants, models
+from threedi_schema import constants, models, ThreediDatabase
+from threedi_schema.beta_features import BETA_COLUMNS, BETA_VALUES
 
 from threedi_modelchecker.checks.other import (
+    BetaColumnsCheck,
+    BetaValuesCheck,
     ChannelManholeLevelCheck,
     ConnectionNodesDistance,
     ConnectionNodesLength,
@@ -15,6 +20,7 @@
     PumpStorageTimestepCheck,
     SpatialIndexCheck,
 )
+from threedi_modelchecker.model_checks import ThreediModelChecker
 
 from . import factories
 
@@ -392,3 +398,70 @@ def test_pumpstation_storage_timestep(
     check = PumpStorageTimestepCheck(models.Pumpstation.capacity)
     invalid = check.get_invalid(session)
     assert len(invalid) == expected_result
+
+
+@pytest.mark.parametrize(
+    "value,expected_result",
+    [
+        (None, 0),  # column not set, valid result
+        (5, 1),  # column set, invalid result
+    ],
+)
+def test_beta_columns(session, value, expected_result):
+    factories.GlobalSettingsFactory(vegetation_drag_settings_id=value)
+    check = BetaColumnsCheck(models.GlobalSetting.vegetation_drag_settings_id)
+    invalid = check.get_invalid(session)
+    assert len(invalid) == expected_result
+
+
+@pytest.mark.parametrize(
+    "value,expected_result",
+    [
+        (
+            constants.BoundaryType.RIEMANN,
+            0,
+        ),  # column not in beta columns, valid result
+        (
+            constants.BoundaryType.GROUNDWATERDISCHARGE,
+            1,
+        ),  # column in beta columns, invalid result
+    ],
+)
+def test_beta_values(session, value, expected_result):
+    beta_values = [
+        constants.BoundaryType.GROUNDWATERLEVEL,
+        constants.BoundaryType.GROUNDWATERDISCHARGE,
+    ]
+    factories.BoundaryConditions1DFactory(boundary_type=value)
+    check = BetaValuesCheck(
+        column=models.BoundaryCondition1D.boundary_type, values=beta_values
+    )
+    invalid = check.get_invalid(session)
+    assert len(invalid) == expected_result
+
+
+@pytest.mark.skipif(
+    condition=(not BETA_COLUMNS and not BETA_VALUES),
+    reason="requires beta features to be defined in threedi-schema to run",
+)
+@pytest.mark.parametrize(
+    "allow_beta_features, no_checks_expected",
+    [
+        (False, False),
+        (True, True),
+    ],
+)
+def test_beta_features_in_server(threedi_db, allow_beta_features, no_checks_expected):
+    with mock.patch.object(ThreediDatabase, "schema"):
+        model_checker = ThreediModelChecker(
+            threedi_db, allow_beta_features=allow_beta_features
+        )
+    model_beta_checks = [
+        check
+        for check in model_checker.config.checks
+        if type(check) in [BetaColumnsCheck, BetaValuesCheck]
+    ]
+    if no_checks_expected:
+        assert len(model_beta_checks) == 0
+    else:
+        assert len(model_beta_checks) > 0