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

Reproject schematisation #160

Open
wants to merge 99 commits into
base: master
Choose a base branch
from

Conversation

margrietpalm
Copy link
Contributor

@margrietpalm margrietpalm commented Dec 24, 2024

Migrate schema such that all geometries in the schematisation have the model CRS. Migrations without epsg_code are not permitted and will raise. To allow for migrations of empty schematisations, as used in tests, custom_epsg_code is added as an argument to the upgrade function.

Added properties to retrieve the epsg_code and source object of that code from the schematisation (used by modelchecker and grid-builder).

To properly set all spatial indices the following is necessary

    op.execute(sa.text(f"SELECT RecoverGeometryColumn('{table_name}', "
                       f"'geom', {srid}, '{geom_type}', 'XY')"))
    op.execute(sa.text(f"SELECT CreateSpatialIndex('{table_name}', 'geom')"))
    op.execute(sa.text(f"SELECT RecoverSpatialIndex('{table_name}', 'geom')"))

Unfortunately, this triggers a bunch of warnings:

updateTableTriggers: "table "idx_channel_geom" already exists"
updateTableTriggers: "table "idx_connection_node_geom" already exists"
updateTableTriggers: "table "idx_cross_section_location_geom" already exists"
updateTableTriggers: "table "idx_culvert_geom" already exists"
updateTableTriggers: "table "idx_orifice_geom" already exists"
updateTableTriggers: "table "idx_pipe_geom" already exists"
updateTableTriggers: "table "idx_pump_geom" already exists"
updateTableTriggers: "table "idx_pump_map_geom" already exists"
updateTableTriggers: "table "idx_weir_geom" already exists"
updateTableTriggers: "table "idx_windshielding_1d_geom" already exists"

I played around with the RecoverGeometryColumn, CreateSpatialIndex and RecoverSpatialIndex but I could not find a combination or order that resulted in valid spatial indices without these warnings. A fresh pair of eyes is highly appreciated.

Note: this will eventually be merged on master, but current work is based on schema leftovers that will be merged with master before this one

@margrietpalm margrietpalm changed the title WIP: Reproject schematisation Reproject schematisation Jan 6, 2025
…ns/threedi-schema into margriet_111_reproject_schematisation
@margrietpalm margrietpalm requested a review from elisalle January 6, 2025 14:19
Base automatically changed from margriet_schema_300_leftovers to master January 8, 2025 06:38
or self.get_version() < 222
or self.get_version() > 229
):
raise ValueError(f"Cannot set epgs code for revision {self.get_version()}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

epgs = epsg

if get_schema_version() > 229:
_upgrade_database(work_db, revision="head", unsafe=True)
with work_db.get_session() as session:
session.execute(text("DELETE FROM model_settings;"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't delete all model settings!

raise InvalidSRIDException(srid_str[0], "the epsg_code must be an integer")
unit, is_projected = get_crs_info(srid)
if unit != "metre":
raise InvalidSRIDException(srid, f"the CRS must be in meters, not {unit}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use metre instead of meter

# This gives a bunch of warnings but seems to be needed to fix spatialite stuff
op.execute(sa.text(f"SELECT RecoverGeometryColumn('{table_name}', "
f"'geom', {srid}, '{geom_type}', 'XY')"))
op.execute(sa.text(f"SELECT CreateSpatialIndex('{table_name}', 'geom')"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

consider skipping for columns that give warnings

@@ -282,6 +282,7 @@ def test_columns_added_tables(self, schema_upgraded):
cols_schema = get_columns_from_schema(schema_upgraded, table)
assert cols_sqlite == cols_schema

@pytest.mark.skip(reason="This test is broken by upgrade to 230")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also check test below!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant