Specific process for geometries with Z or M coordinate with SpatiaLite dialect

[sdp5]

Description

Though most of the dialects support geom_type ending with Z in SQL queries, it seems not working with SpatiaLite.
Hence replacing Z with a blank in SQL queries.

Added a test to cover this behaviour.

Checklist

This pull request is:

• A short code fix
  • Fixes: Assistanze in using 3D Geometries #505

@adrien-berchet Please take an initial look, thanks!

[adrien-berchet]

Thank you very much for this proposal @sdp5 !
I'm going to setup a more general test to be sure we cover all cases and then we'll see how bind_processor_process() can handle all these cases with your fix.

[sdp5]

Agree, getting your point @adrien-berchet and thank you for setting up the tests. 👍🏻

Allow me some more time testing, will update the PR shortly.

[sdp5]

especially the M cases, like POINT M or POINT ZM, it should be added in the test

I guess support for 'M' is coming in shapely.

>>> from shapely import wkt
>>> g = wkt.loads('POINT M (0 0 5)')
>>> g
<POINT Z (0 0 5)>

Perhaps 'M' and 'ZM' support could be put on the TODO-list?

Plus with 'ZM' ... shapely returns ValueError: The ordinate (last) dimension should be 2 or 3, got 4!

[adrien-berchet]

especially the M cases, like POINT M or POINT ZM, it should be added in the test

I guess support for 'M' is coming in shapely.

>>> from shapely import wkt
>>> g = wkt.loads('POINT M (0 0 5)')
>>> g
<POINT Z (0 0 5)>

Perhaps 'M' and 'ZM' support could be put on the TODO-list?

Plus with 'ZM' ... shapely returns ValueError: The ordinate (last) dimension should be 2 or 3, got 4!

Hmmm ok
Well, we can still process the WKT strings for M and ZM cases, even though Shapely does not support it yet. So it will still fix the cases with raw strings and WKTElements, only the WKBElements will fail.

[adrien-berchet]

I'm not able to reproduce the CI failure locally, even in the test Docker container 🤔
We'll have to investigate that.

[sdp5]

ran some tests quickly with postgis/postgis:15-3.3 image.
and found a few tests failing for postgresql dialect, MULTIPOINT geom_type.

FAILED tests/test_functional.py::TestInsertionCore::test_insert_all_geom_types[postgresql-Multi Point ZM]
AssertionError: assert 'MULTIPOINTZM((1234),(5678))' == 'MULTIPOINTZM(1234,5678)'

we can fix that by adding following in test_insert_all_geom_types

if dialect_name == 'postgresql' and 'MULTIPOINT' in geom_type:
      # formats wkt to wrap tuple elements in brackets,
      # for example '(1 2, 3 4)' to '((1 2), (3 4))'.
      wkt = "({})".format(
           ", ".join(map(lambda x: '({})'.format(x.strip()), wkt[1:-1].split(',')))
      )

Can we use this image in test_and_publish.yml?

[sdp5]

The only nice thing to have would be to update the doc to specify which cases are supported and which are not.

@adrien-berchet for docs, should we make it a part of core_tutorial.rst#Insertions ?

[adrien-berchet]

ran some tests quickly with postgis/postgis:15-3.3 image. and found a few tests failing for postgresql dialect, MULTIPOINT geom_type.

FAILED tests/test_functional.py::TestInsertionCore::test_insert_all_geom_types[postgresql-Multi Point ZM]
AssertionError: assert 'MULTIPOINTZM((1234),(5678))' == 'MULTIPOINTZM(1234,5678)'

we can fix that by adding following in test_insert_all_geom_types

if dialect_name == 'postgresql' and 'MULTIPOINT' in geom_type:
      # formats wkt to wrap tuple elements in brackets,
      # for example '(1 2, 3 4)' to '((1 2), (3 4))'.
      wkt = "({})".format(
           ", ".join(map(lambda x: '({})'.format(x.strip()), wkt[1:-1].split(',')))
      )

I'm surprised they changed this behavior, that's quite annoying.

Can we use this image in test_and_publish.yml?

Yeah, let's go for a newer version if it does not break anything (except the MULTIPOINT thing ofc).

Thanks for this!

[adrien-berchet]

The only nice thing to have would be to update the doc to specify which cases are supported and which are not.

@adrien-berchet for docs, should we make it a part of core_tutorial.rst#Insertions ?

Yeah that looks relevant.

[adrien-berchet]

For the CI I see that you need to add suppress_warnings = ["config.cache"] in doc/conf.py. There is also an issue with the postgis_raster extension that can't be created during setup, I don't why.

[adrien-berchet]

Looks good this time 🥳
Do you think it's ok to be merged now or do you still have some tests to do @sdp5 ?

[sdp5]

🎉 thanks, please go ahead merging @adrien-berchet !

[adrien-berchet]

Thank you very much for this work @sdp5 !!!