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

Requirement 13 Information Incomplete / Test Incomplete #444

Closed
ajanett opened this Issue May 31, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@ajanett

ajanett commented May 31, 2018

Requirement 13 currently states: A GeoPackage file SHALL include a gpkg_contents table per table Contents Table Definition and gpkg_contents Table Definition SQL.

Note, it refers to "Contents Table Definition", and this is Table 4. The very last row of Table 4 discusses srs_id and it has this information (and note the words SHALL):
Spatial Reference System ID:
gpkg_spatial_ref_sys.srs_id; when data_type is features, SHALL also match
gpkg_geometry_columns.srs_id; When data_type is tiles, SHALL also match
gpkg_tile_matrix_set.srs_id

Req 13 test information only provides the tests on the table definition and there are no tests specified for this consistency requirement on srs_id. The consistency checks between the srs_id in gpkg_spatial_ref_sys, gpkg_geometry_columns, gpkg_tile_matrix_set and the gpkg_contents all need to be specified more clearly as requirements AND specified within one or more tests.

@bosborn

This comment has been minimized.

Contributor

bosborn commented May 31, 2018

I brought something up similar to this in #233 but the SWG voted to make no changes.

@ajanett

This comment has been minimized.

ajanett commented May 31, 2018

Yes, thank you. There are similarities with #233. what I am looking for is not removal of the requirement, but I would like the specification to optionally clarify it, and definitely add the required tests for it. That was one of the options presented in #233, and perhaps not all of the options were discussed.

It certainly seems strange to test to ensure consistency of the srs_id between the feature geometry and gpkg_geometry_column srs_id (Req 33); but then to ignore consistency issues of the srs_id between gpkg_geometry_columns and gpkg_contents (stated in the table referenced by requirement 13, but not included in req 13 test or any other test).

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Jun 13, 2018

This is a gap in the requirements. From a logical perspective, gpkg_geometry_columns and gpkg_tile_matrix depend on gpkg_contents so the requirements have to follow that dependency. We added that bit to the table 4 for additional clarity, but maybe the use of the word "SHALL" was inappropriate there. The text in http://www.geopackage.org/guidance/getting-started.html also is consistent, at least as I read it.

I am working on a fix.

jyutzler added a commit to jyutzler/geopackage that referenced this issue Jun 13, 2018

jyutzler added a commit that referenced this issue Jun 18, 2018

Merge pull request #447 from jyutzler/i444
#444 enforce match between srs_ids
@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Jun 18, 2018

@ajanett are you satisfied that #447 resolves this issue?

@ajanett

This comment has been minimized.

ajanett commented Jun 18, 2018

@jyutzler Yes, I reviewed the changes and they completely resolve the issue. Thank you.

@jyutzler jyutzler closed this Jun 19, 2018

jyutzler added a commit that referenced this issue Jun 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment