Skip to content

Commit

Permalink
Adapt to schema 230 (implicit CRS) (#428)
Browse files Browse the repository at this point in the history
* Change CRS in all tests to 28892

* Disable RasterHasMatchingEPSGCheck (should be fixed in ticket 414)

* Remove geo_query and use explicit queries instead now transformations are no longer needed
  • Loading branch information
margrietpalm committed Jan 2, 2025
1 parent 9e00dcd commit 11fa2b0
Show file tree
Hide file tree
Showing 13 changed files with 185 additions and 316 deletions.
11 changes: 10 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@ Changelog of threedi-modelchecker



2.14.2 (unreleased)
2.16.0 (unreleased)
-------------------

- Adapt to schema 230 where all geometries use the model CRS and model_settings.epsg_code is no longer available
- Remove checks for model_settings.epsg_code (317 and 318)
- Remove usage of epsg 4326 in the tests because this CRS is no longer valid
- Remove no longer needed transformations


2.15.0 (unreleased)
-------------------

- Change minimum python version to 3.9 in pyproject.toml, update test matrix.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ dependencies = [
"Click",
"GeoAlchemy2>=0.9,!=0.11.*",
"SQLAlchemy>=1.4",
"threedi-schema @ git+https://github.com/nens/threedi-schema.git@margriet_schema_300_leftovers"
"threedi-schema @ git+https://github.com/nens/threedi-schema.git@margriet_111_reproject_schematisation"
]

[project.optional-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion threedi_modelchecker/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .model_checks import * # NOQA

# fmt: off
__version__ = '2.14.2.dev0'
__version__ = '2.16.0.dev0'
# fmt: on
24 changes: 0 additions & 24 deletions threedi_modelchecker/checks/geo_query.py

This file was deleted.

17 changes: 8 additions & 9 deletions threedi_modelchecker/checks/location.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
from typing import List, NamedTuple

from sqlalchemy import func
from geoalchemy2.functions import ST_Distance, ST_NPoints, ST_PointN
from sqlalchemy.orm import aliased, Session
from threedi_schema.domain import models

from threedi_modelchecker.checks.base import BaseCheck
from threedi_modelchecker.checks.geo_query import distance


class PointLocationCheck(BaseCheck):
Expand All @@ -32,7 +31,7 @@ def get_invalid(self, session):
self.ref_table,
self.ref_table.id == self.ref_column,
)
.filter(distance(self.column, self.ref_table.geom) > self.max_distance)
.filter(ST_Distance(self.column, self.ref_table.geom) > self.max_distance)
.all()
)

Expand Down Expand Up @@ -72,13 +71,13 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
end_node = aliased(self.ref_table_end)

tol = self.max_distance
start_point = func.ST_PointN(self.column, 1)
end_point = func.ST_PointN(self.column, func.ST_NPoints(self.column))
start_point = ST_PointN(self.column, 1)
end_point = ST_PointN(self.column, ST_NPoints(self.column))

start_ok = distance(start_point, start_node.geom) <= tol
end_ok = distance(end_point, end_node.geom) <= tol
start_ok_if_reversed = distance(end_point, start_node.geom) <= tol
end_ok_if_reversed = distance(start_point, end_node.geom) <= tol
start_ok = ST_Distance(start_point, start_node.geom) <= tol
end_ok = ST_Distance(end_point, end_node.geom) <= tol
start_ok_if_reversed = ST_Distance(end_point, start_node.geom) <= tol
end_ok_if_reversed = ST_Distance(start_point, end_node.geom) <= tol
return (
self.to_check(session)
.join(start_node, start_node.id == self.ref_column_start)
Expand Down
23 changes: 10 additions & 13 deletions threedi_modelchecker/checks/other.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from dataclasses import dataclass
from typing import List, Literal, NamedTuple

from geoalchemy2.functions import ST_Distance, ST_Length
from sqlalchemy import (
and_,
case,
Expand All @@ -19,7 +20,6 @@

from .base import BaseCheck, CheckLevel
from .cross_section_definitions import cross_section_configuration_for_record
from .geo_query import distance, length, transform


class CorrectAggregationSettingsExist(BaseCheck):
Expand Down Expand Up @@ -258,8 +258,7 @@ def get_invalid(self, session):

class ConnectionNodesLength(BaseCheck):
"""Check that the distance between `start_node` and `end_node` is at least
`min_distance`. The coords will be transformed into (the first entry) of
ModelSettings.epsg_code.
`min_distance`.
"""

def __init__(
Expand Down Expand Up @@ -290,7 +289,7 @@ def get_invalid(self, session):
self.to_check(session)
.join(start_node, start_node.id == self.start_node)
.join(end_node, end_node.id == self.end_node)
.filter(distance(start_node.geom, end_node.geom) < self.min_distance)
.filter(ST_Distance(start_node.geom, end_node.geom) < self.min_distance)
)
return list(q.with_session(session).all())

Expand Down Expand Up @@ -324,7 +323,7 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
f"""SELECT *
FROM connection_node AS cn1, connection_node AS cn2
WHERE
distance(cn1.geom, cn2.geom, 1) < :min_distance
distance(cn1.geom, cn2.geom) < :min_distance
AND cn1.ROWID != cn2.ROWID
AND cn2.ROWID IN (
SELECT ROWID
Expand Down Expand Up @@ -548,10 +547,10 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
linestring = models.Channel.geom
tol = self.min_distance
breach_point = func.Line_Locate_Point(
transform(linestring), transform(func.ST_PointN(self.column, 1))
linestring, func.ST_PointN(self.column, 1)
)
dist_1 = breach_point * length(linestring)
dist_2 = (1 - breach_point) * length(linestring)
dist_1 = breach_point * ST_Length(linestring)
dist_2 = (1 - breach_point) * ST_Length(linestring)
return (
self.to_check(session)
.join(models.Channel, self.table.c.channel_id == models.Channel.id)
Expand All @@ -577,10 +576,8 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:

# First fetch the position of each potential breach per channel
def get_position(point, linestring):
breach_point = func.Line_Locate_Point(
transform(linestring), transform(func.ST_PointN(point, 1))
)
return (breach_point * length(linestring)).label("position")
breach_point = func.Line_Locate_Point(linestring, func.ST_PointN(point, 1))
return (breach_point * ST_Length(linestring)).label("position")

potential_breaches = sorted(
session.query(self.table, get_position(self.column, models.Channel.geom))
Expand Down Expand Up @@ -776,7 +773,7 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
self.table.c.id,
self.table.c.area,
self.table.c.geom,
func.ST_Area(transform(self.table.c.geom)).label("calculated_area"),
func.ST_Area(self.table.c.geom).label("calculated_area"),
).subquery()
return (
session.query(all_results)
Expand Down
10 changes: 4 additions & 6 deletions threedi_modelchecker/checks/raster.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,10 @@ class RasterHasMatchingEPSGCheck(BaseRasterCheck):
"""Check whether a raster's EPSG code matches the EPSG code in the global settings for the SQLite."""

def get_invalid(self, session):
epsg_code_query = session.query(models.ModelSettings.epsg_code).first()
if epsg_code_query is not None:
self.epsg_code = epsg_code_query[0]
else:
self.epsg_code = None
return super().get_invalid(session)
# TODO: replace this check with check that ensures that all EPSG
# in a schematisation match (ticket 414)
return []
# return super().get_invalid(session)

def is_valid(self, path: str, interface_cls: Type[RasterInterface]):
with interface_cls(path) as raster:
Expand Down
25 changes: 6 additions & 19 deletions threedi_modelchecker/config.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from typing import List

from geoalchemy2 import functions as geo_func
from sqlalchemy import and_, func, or_, true
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 (
AllEqualCheck,
BaseCheck,
Expand Down Expand Up @@ -1068,7 +1068,7 @@ def is_none_or_empty(col):
error_code=202,
level=CheckLevel.WARNING,
column=table.id,
invalid=Query(table).filter(geo_query.length(table.geom) < 5),
invalid=Query(table).filter(geo_func.ST_Length(table.geom) < 5),
message=f"The length of {table.__tablename__} is very short (< 5 m). A length of at least 5.0 m is recommended to avoid timestep reduction.",
)
for table in [models.Channel, models.Culvert]
Expand Down Expand Up @@ -1369,8 +1369,8 @@ def is_none_or_empty(col):
invalid=Query(models.ExchangeLine)
.join(models.Channel, models.Channel.id == models.ExchangeLine.channel_id)
.filter(
geo_query.length(models.ExchangeLine.geom)
< (0.8 * geo_query.length(models.Channel.geom))
geo_func.ST_Length(models.ExchangeLine.geom)
< (0.8 * geo_func.ST_Length(models.Channel.geom))
),
message=(
"exchange_line.geom should not be significantly shorter than its "
Expand All @@ -1384,7 +1384,7 @@ def is_none_or_empty(col):
invalid=Query(models.ExchangeLine)
.join(models.Channel, models.Channel.id == models.ExchangeLine.channel_id)
.filter(
geo_query.distance(models.ExchangeLine.geom, models.Channel.geom) > 500.0
geo_func.ST_Distance(models.ExchangeLine.geom, models.Channel.geom) > 500.0
),
message=("exchange_line.geom is far (> 500 m) from its corresponding channel"),
),
Expand Down Expand Up @@ -1455,7 +1455,7 @@ def is_none_or_empty(col):
invalid=Query(models.PotentialBreach)
.join(models.Channel, models.Channel.id == models.PotentialBreach.channel_id)
.filter(
geo_query.distance(
geo_func.ST_Distance(
func.PointN(models.PotentialBreach.geom, 1), models.Channel.geom
)
> TOLERANCE_M
Expand Down Expand Up @@ -1579,19 +1579,6 @@ def is_none_or_empty(col):
column=models.ModelSettings.manhole_aboveground_storage_area,
min_value=0,
),
QueryCheck(
error_code=317,
column=models.ModelSettings.epsg_code,
invalid=CONDITIONS["has_no_dem"].filter(models.ModelSettings.epsg_code == None),
message="model_settings.epsg_code may not be NULL if no dem file is provided",
),
QueryCheck(
error_code=318,
level=CheckLevel.WARNING,
column=models.ModelSettings.epsg_code,
invalid=CONDITIONS["has_dem"].filter(models.ModelSettings.epsg_code == None),
message="if model_settings.epsg_code is NULL, it will be extracted from the DEM later, however, the modelchecker will use ESPG:28992 for its spatial checks",
),
QueryCheck(
error_code=319,
column=models.ModelSettings.use_2d_flow,
Expand Down
5 changes: 3 additions & 2 deletions threedi_modelchecker/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ def threedi_db(tmpdir_factory):
shutil.copyfile(data_dir / "empty.sqlite", tmp_sqlite)
db = ThreediDatabase(tmp_sqlite)
schema = ModelSchema(db)
schema.upgrade(backup=False, upgrade_spatialite_version=False)
schema.upgrade(
backup=False, upgrade_spatialite_version=False, custom_epsg_code=28992
)
schema.set_spatial_indexes()
return db

Expand All @@ -40,7 +42,6 @@ def session(threedi_db):
:return: sqlalchemy.orm.session.Session
"""
s = threedi_db.get_session(future=True)

factories.inject_session(s)
s.model_checker_context = LocalContext(base_path=data_dir)

Expand Down
Loading

0 comments on commit 11fa2b0

Please sign in to comment.