Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Adapt to schema 230 (implicit CRS) #428

Merged
merged 6 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

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

from geoalchemy2.functions import ST_Distance
from sqlalchemy import func
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 +32,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 @@ -75,10 +75,10 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
start_point = func.ST_PointN(self.column, 1)
end_point = func.ST_PointN(self.column, func.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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The third argument, use_ellipsoid, is only suported for long/lat coordinates and will return None's for projected. Because from now on only projected should be allowed, this argument is removed.

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
Loading