Geopackage support

[margrietpalm]

Support geopackage sqlite dialect:

• Support reading geopackage files (.gpkg) files, note that per specification geopackes are required to have this extension
• Add functionality to convert spatialite to geopackage: schema.upgrade(convert_to_geopackage=True)
• Run all tests using the latest schema with geopackage instead of spatialiate

Note that the default settings is schema.upgrade(convert_to_geopackage=False). Hence without any changes to threedi-api the geopackage will not be used.

Note2 geopackage conversion requires ogr2ogr >= 3.4, which is part of gdal-bin. If ogr2ogr is not properly installed or a too old version is present, the conversion is not done and a warning is raised.

Local tests can be done by installing threedi-schema==0.219.2.dev1

[margrietpalm]

Tests falen voor oude python en ubuntu

• Falende tests:
  • Ubuntu 20.04:
  • sqlite 3.31.1 2020-01-27 19:55:54 3bfa9cc97da10598521b342961df8f5f68c7388fa117345eeb516eaa837balt1
  • GDAL 3.0.4, released 2020/01/28
  • python 3.8 of 3.9
  • sqlalchemy==1.4.44 alembic==1.8.* geoalchemy2==0.14.0
• Werkende tests:
  • Ubuntu 22.04
  • sqlite 3.37.2 2022-01-06 13:25:41 872ba256cbf61d9290b571c0e6d82a20c224ca3ad82971edc46b29818d5dalt1
  • GDAL 3.4.1, released 2021/12/27
  • Python
    • 3.10 + sqlalchemy==1.4.44 alembic==1.8.* geoalchemy2==0.14.0
    • 3.11 + sqlalchemy==2.0.24 alembic==1.13.1 geoalchemy2==0.14.3
    • 3.12 + sqlalchemy==2.0.* alembic==1.13.* geoalchemy2==0.14.*

Gerelateerde problemen (allemaal qgis):

• https://gis.stackexchange.com/questions/400740/unique-constraint-failed-error-when-saving-geopackage-in-qgis
• Shape to Geopackage: Creation of field fid failed (OGR error: Wrong field type for fid) qgis/QGIS#25795

[jpprins1]

Overall: looks very good!

I've added some comments with some (minor) things to address/look at.

[jpprins1]

Suggested change

	self._path = path
	self.path = path if isinstance(path, Path) else Path(path)

Is shorter and allows to drop the path property and @path.setter property

[margrietpalm]

This breaks the current working of _path and path. I introduced the setter to control the type of ThreediDatabase.path because it is set from multiple libraries, sometimes with a Path and sometimes with a string. With this implementation self._path is always a string and self.path will return a path; and setting ThreediDatabase.path will ensure that both self._path and self.path are set properly.

It would be nicer to just have self.path and make is a Path (and add type hints). However, that also involves threedi-api; and I feel that this PR is complex enough as it is.

Note that self._path is now also cast to a string in the constructor 9fd2fe6.

[jpprins1]

Ok, I saw in the test that a Path instance was passed as parameter to ThreediDatabase(xxxx) # constructor
My suggestion also works if a string is passed. It's just converted into a Path.

The only thing that is not backwards-compatible is:

1. db = ThreediDatabase("/tmp/blaat.gpkg") and directly getting db.path as a string. (note: same new behaviour as your property introduced)
2. db = ThreediDatabase("/tmp/no_really_the.gpkg") ; db.path = "/tmp/the_real.gpkg". This is going to crash, but can be fixed by "path = self.path if isinstance(self.path, Path) else Path(self.path)" in the get_engine method.

Maybe 2 is actually the better solution. Convert it there to a Path instance if it not already the case:

path = self.path if isinstance(self.path, Path) else Path(self.path)

The property, setter and self._path can all be dropped and self.path stays exactly the same as what the user has put into the class constructor. And changing it after initialisation also works.

[margrietpalm]

Modified in d5f8545. Note that using a Path in get_engine is necessary to handle the in-memory sqlite. This is because str(Path("")) and str("") don't match, but Path("") and Path(str("")) do.

[jpprins1]

👍