-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix v2 SRS import / upgrade / export #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of previous commit, SRS definitions were assumed to be EPSG. Now, we actually parse the WKT in the SRS definition and extract the authority name and id from it.
Can you expand on the logic used, and the flow of IDs/definitions through various states?
eg. for a GPKG with crs organization=EPSG,organization_coordsys_id=12345,description=My Lands
and WKT that includes no authority codes or descriptions, how does this get parsed/persisted/reloaded/applied?
{ | ||
"srs_name": "Unknown CRS", | ||
"definition": "", | ||
"organization": "EPSG", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EPSG:0
isn't a thing. We should follow the approach of http://www.geopackage.org/spec/#r11 here, and use NONE
if we really need a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This preserves the previous import behaviour where we always assumed "EPSG", but srs_id would return 0 if we couldn't find a SpatialReference. But, happy to change it to "NONE"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GPKG specifically has set values (NONE:0 and NONE:-1) that must be in every GPKG, but I think internally we should probably just not have a crs (& wkt) set? And the working copies can decide how to represent that (eg. PostGIS uses 0)
"""Given a osgeo SpatialReference, generate a sensible name for it.""" | ||
auth_name = spatial_ref.GetAuthorityName(None) | ||
auth_code = spatial_ref.GetAuthorityCode(None) | ||
return f"{auth_name}:{auth_code}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar q around what is returned for unknown/custom CRS
] | ||
|
||
|
||
DEFAULT_SRS_STR = "EPSG:0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EPSG:0 isn't a thing
As of previous commit, SRS definitions were assumed to be EPSG. Now, we actually parse the WKT in the SRS definition and extract the authority name and id from it.
Uh oh: fixing this ended up being a refactoring change also. I have begun the work of moving GPKG specific things into gpkg_adapter (renamed from dataset2_gpkg), starting with "gpkg_spatial_ref_sys". Now that GPKG is not our model, importers / datasets should not have an accessor for "gpkg_spatial_ref_sys" as their main interface. (Right now they still have this accessor, but providing it is delegated to the adapter class.) Instead, the main interface going forward is a list of (srs_name, srs_definition_in_wkt) tuples, which is basically what is stored in V2 - which makes sense as the V2 design is supposed to be simple and all-purpose.
This PR is the start of an inversion - instead of importers first importing to GPKG, and then this is adapted to V2, we should import to V2 - that is, our new model - and have adapters to adapt that to gpkg. See gpkg_adapter. More changes along these lines will come as V2 supercedes V1 entirely and V1-centric structure is removed from codebase, although this cleanup might be well into V2 development. For instance, eventually code to convert OGR types to GPKG types should go away, and instead we'll convert OGR types to V2 types and then, if necessary, from V2 to GPKG.
Import sources and datasets continue to converge - still hope to define this interface properly at some point and make them all implement it, move secondary functionality (eg GPKG meta item generation) into an adapter that works on this interface.