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

CRS WKT given in example 5.12 is invalid #566

Open
observingClouds opened this issue Nov 13, 2024 · 2 comments · May be fixed by #567
Open

CRS WKT given in example 5.12 is invalid #566

observingClouds opened this issue Nov 13, 2024 · 2 comments · May be fixed by #567
Labels
CF1.12? We might conclude this issue in time for CF1.12 defect Conventions text meaning not as intended, misleading, unclear, has typos, format or language errors new contributor This issue was worked on by new contributors to the CF conventions

Comments

@observingClouds
Copy link

observingClouds commented Nov 13, 2024

Title

Chapter 5: Example 5.12 with invalid crs wkt

Moderator

TBD

Requirement Summary

The crs_wkt attribute in example 5.12 needs slight adjustments to become a valid crs wkt string.

Technical Proposal Summary

  1. Decide on example fixture (minimal or conform with WKT:2015)
  2. Update example 5.12

Benefits

The CF-conventions are a trusted source for well defined standards. Ensuring the CRS WKT string follows the WKT specifications shows that CF respects underlying specifications. Examples are probably often copied and adapted to the users needs, ensuring the examples follow all standards, ensures that more downstream products will follow those standards as well.

Status Quo

The crs wkt string of example 5.12 fails, e.g. when reading via the pyproj library and the underlying proj library.

Associated pull request

The suggested minimal changes can be viewed at observingClouds@72e43de

Detailed Proposal

Upon testing the interpretation of the projection information given in several cf-convention examples, it turned out that the crs_wkt in example 5.12 cannot be read:

CRSError: Invalid projection: COMPOUNDCRS ["OSGB 1936 / Br...: (Internal Proj Error: proj_create: Missing BASEGEODCRS / BASEGEOGCRS / GEOGCS node)

Raising this issue with the used library to read the crs_wkt, confirmed that the issue is not a library issue but an invalid crs wkt string

@observingClouds observingClouds added the defect Conventions text meaning not as intended, misleading, unclear, has typos, format or language errors label Nov 13, 2024
@observingClouds observingClouds changed the title CRS WKT given example 5.12 is invalid CRS WKT given in example 5.12 is invalid Nov 13, 2024
@JonathanGregory
Copy link
Contributor

Dear Hauke @observingClouds

Thanks very much for testing the examples and reporting the error along with a correction. That's ideal! If you are willing to make a PR from your proposed correction and attach it to this issue, it will help us implement it in the imminent CF 1.12 release. As a defect, this issue will be agreed in three weeks, on 3rd December, if no-one raises any concerns.

Best wishes

Jonathan

@JonathanGregory JonathanGregory added CF1.12? We might conclude this issue in time for CF1.12 new contributor This issue was worked on by new contributors to the CF conventions labels Nov 13, 2024
@observingClouds observingClouds linked a pull request Nov 13, 2024 that will close this issue
3 tasks
@JonathanGregory
Copy link
Contributor

Thanks for the preparing the PR, Hauke. We'll leave it open until 3rd December, according to the usual rules for updates.

@JonathanGregory JonathanGregory linked a pull request Nov 14, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CF1.12? We might conclude this issue in time for CF1.12 defect Conventions text meaning not as intended, misleading, unclear, has typos, format or language errors new contributor This issue was worked on by new contributors to the CF conventions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants