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

Adds support for IAU codes. #12

Merged
merged 34 commits into from
Jul 22, 2024
Merged

Adds support for IAU codes. #12

merged 34 commits into from
Jul 22, 2024

Conversation

jlaura
Copy link
Contributor

@jlaura jlaura commented Oct 17, 2022

Fixes #8.

This PR includes changes necessary to support IAU codes in the proj extension. To support non-EPSG codes, it is necessary to split the authority and the code into two separate fields. The PR adds proj:authority with a default of epsg and proj:code. The PR maintains the proj:epsg field for backwards compatibility.

I ticked the version of the extension to 2.0.0 since this should likely be a breaking change where proj:epsg is simply deprecated, but I am not 100% how the maintainers prefer to version the extension spec.

Happy to update, make changes, etc.

The follow on to this PR are PRs across the STAC ecosystem to update reading projections in the form authority:code instead of assuming EPSG.

@m-mohr
Copy link
Contributor

m-mohr commented Oct 24, 2022

It's not ideal to break/deprecate proj:epsg as it's widely used. You could set proj:epsg to null and instead use projjson or wkt2 for your use cases, right? Are you using other proj fields like bbox, geometry, shape or transform?
I was wondering whether you could add ssys:iau to the ssys extension?

@jlaura
Copy link
Contributor Author

jlaura commented Oct 24, 2022

I agree it is not ideal. I would suggest that hard coding to that authority is not ideal either. Even terrestrially, other authorities exist. We are using many other proj extension fields (box, transform, etc)., so true integration into the standard would be ideal.

On syss:iau: Yes and no. Sure, we could use a different extension to encode the projection, but then that requires that we write custom clients that know to look in a non-standard place for the projection information. That sees counter to the goal of integrating into the STAC ecosystem.

@jlaura
Copy link
Contributor Author

jlaura commented Dec 6, 2022

@m-mohr It's been a little over a month and I wonder what alternatives we might be able to explore to be able to have explicit support for non-EPSG projections in the proj extension. The extension is so core to STAC and the STAC ecosystem, that I would love to find a solution moving forward for expanded authority support!

@m-mohr
Copy link
Contributor

m-mohr commented Dec 6, 2022

I'm not the authority here, others "own" this repository. I didn't get from your response whether WKT2 or PROJJSON with EPSG code set to null works for you?

I think ideally such a change would be discussed in one of the STAC community meetings (every other Monday, we are happy to invite everyone).

@jlaura
Copy link
Contributor Author

jlaura commented Dec 6, 2022

@m-mohr Sounds great! I would be more than happy to attend and discuss. I know we (USGS Astrogeolgoy) attended once before, but I can not recall the meeting info (e.g., time / link). Would you mind posting or emailing me ([email protected])?

And apologies, I was not attempting to ignore your question. Here is an example where the EPSG code is null and the WKT2 string is presented. (and thank you for all your hard work on the Stac Browser!) My concern is, are clients using these fields as well or are they using the EPSG code.

Additionally, the WKT2 string is (via GDAL) encoding an incorrect authority / code 🤦‍♂️. So, work to do there as well!

@jlaura
Copy link
Contributor Author

jlaura commented Dec 7, 2022

You learn something new every day - the EPSG 9122 in that code not for a projection. It is the code for degrees as explain by this SO post. I likely should update these STAC records to use this wkt2 definition as it is more complete.

The OGC should be proxying links to these approved International Astronomical Union (IAU) codes sometime soon as the IAU is the authority on planetary projections / projection codes.

@jlaura
Copy link
Contributor Author

jlaura commented Dec 11, 2022

Here is a great OGC discussion where they are working on ratifying the IAU project codes to be linked via opengis.net. Sounds like this might be ratified by the 20th of this month. A few of us are working on getting morecantile (our version being aptly called planetcantile) updated with TileMatrixSets for all of these bodies. This way we can use titiler to handle our planetary TMSs.

I am posting just as an example of the further integration of the IAU codes into the corpus or projections and how getting those codes integrated makes it so that the rest of the ecosystem (e.g., morecantile and up the stack) are better supported. We desperately want to be able to work as 'out-of-the-box' as possible in the web geospatial stack.

(Context: Right now morecantile fails when attempting to read a planetary TMS with a non-EPSG code from a JSON specification file. This might be an issue all the way down in proj/pyproj; I haven't had time to explore.)

@jlaura
Copy link
Contributor Author

jlaura commented Mar 10, 2023

I believe I ran into this issue again today. I have a STAC API deploy with data that uses the proj extension. The data use the proper (OGC adopted) IAU projection codes. I also have a titiler deploy with TMSs that support the aforementioned IAU codes. I do not see a means to match the STAC item (proj:epsg is naturally null) with the proper TMS endpoint (as one needs to define a TileMatrixSetId that corresponds to the projection code).

Here is an example STAC file. The WKT2 information is correct, but does not include the authority information (perhaps GDAL is stripping it out?). The projjson information is what comes out of GDAL on ingest of these data.

Since we have 7 major bodies (Earth omitted) and dozens of minor bodies to support, it would be great to hear ideas about how this extension might support non-EPSG projections (the solution I proposed is one of many I suspect). I am hesitant to push the projection into the ssys extension because this extension is for projection information. It seems like an anti-pattern to embed projection information across metadata extensions.

P.S. I quite appreciate all of the work that continues to go into this ecosystem. So, none of the above is intend to come across as angry. I absolutely understand that the vast majority of the user community used EPSG codes. If we had planetary EPSG codes, I would be using them too! I just want to get/keep a conversation rolling so that planetary can make use of the great ecosystem without hacking on solutions.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
json-schema/schema.json Outdated Show resolved Hide resolved
json-schema/schema.json Outdated Show resolved Hide resolved
jlaura and others added 6 commits April 3, 2023 15:34
* Add more examples, clarify/fix `proj:epsg` requirement stac-extensions#7, align repo with template

* Don't require proj:epsg as discussed in the STAC meeting

* Clarification

* Recommendation around thumbnails

* Update README.md

Co-authored-by: Pete Gadomski <[email protected]>

---------

Co-authored-by: Pete Gadomski <[email protected]>
@jlaura
Copy link
Contributor Author

jlaura commented Apr 3, 2023

@philvarner CI needs a run on this. I'll take another look when I am fresh in the AM as well. I'm still getting used to reading JSON schema files!

@m-mohr
Copy link
Contributor

m-mohr commented Apr 10, 2023

I'll try to summarize from todays discussion:

  • Keep peoj:epsg as is, but deprecate
  • Require proj:code if proj:authority is given and vice-versa
  • Provide code and authority as strings
  • no authority enum in schema, but add a table in the readme with a proposed mapping

CHANGELOG.md Outdated Show resolved Hide resolved
@jlaura
Copy link
Contributor Author

jlaura commented Dec 5, 2023

@m-mohr What do you think?? Big green button time?

README.md Outdated
@@ -37,7 +37,7 @@ The fields in the table below can be used in these parts of STAC documents:

| Field Name | Type | Description |
| ---------------- | ------------------------ | ----------- |
| proj:epsg | integer\|null | [EPSG code](http://www.epsg-registry.org/) of the datasource |
| proj:code | string\|null | Authority and specific code of the data source (e.g., ["EPSG:####"](https://epsg.org/), ["IAU:#####"](http://www.opengis.net/def/crs/IAU/2015)) |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example uses IAU instead of IAU_2015 like other ones.

README.md Show resolved Hide resolved
Copy link

@philvarner philvarner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authority issues

@m-mohr
Copy link
Contributor

m-mohr commented Dec 18, 2023

@jlaura Happy to merge once Phil's comments are solved.

@m-mohr
Copy link
Contributor

m-mohr commented Dec 19, 2023

I have one question that recently came up: With this PR proj:epsg is eplicitly disallowed, so having it in an implementation would throw an error in validation once you migrate from version 1.x to 2.x. I'm wondering whether that's a bit too strict and breaking. Should we allow a migration period where for backward compatibility proj:epsg can be provided in addition to proj:code?

We could for example release version 1.2 with proj:epsg and proj:code as a intermediate version people could implement while also releasing 2.0 without proj:epsg at the same time. This would allow people to implement proj:epsg and proj:code at the same time in at least one version. I would hope that this would encourage adoption as it doesn't break related systems directly.

Thoughts? @jlaura @gadomski @philvarner @matthewhanson @emmanuelmathot

@gadomski
Copy link
Member

We could for example release version 1.2 with proj:epsg and proj:code as a intermediate version people could implement while also releasing 2.0 without proj:epsg at the same time.

I've been thinking about this a bit, and @jsignell likely has thoughts as well. My initial instinct is that "transitional" versions are not very useful for extensions, because (at least for the big providers) re-processing everything to update an extension version is a big lift w/ very little payoff.

From a tooling side, I think it'd be simpler to just go straight to a breaking v2.0 and handle that in code, but again @jsignell has been working on the PySTAC implementation and has some actual evidence-based opinions, as opposed to my hand-waving.

@m-mohr
Copy link
Contributor

m-mohr commented Dec 19, 2023

@gadomski That's the static STAC perspective, which is valid of course. I'd like to add that from an API pespective (with a database) it's easy to just implement a bit of code that adds the proj:code in addition to the potential epsg code field in the database and as such can offer both at the same time to increase client/downstream support.

@jsignell
Copy link

I tend to agree that a transitional version doesn't get us that much. In the case of pystac it is always going to try to migrate existing objects and write new objects with the most recent version of an extension that it knows about. So releasing proj v1.2 and v2.0 at the same time will have very little impact on the code changes.

Also having both adds a bit of ambiguity. Is the idea that there would be validation to ensure that in v1.2 proj:epsg and proj:code are always in sync?

@m-mohr
Copy link
Contributor

m-mohr commented Dec 19, 2023

I'd need to check JSON Schema options, but if possible it would be good to validate that proj:epsg and proj:code are aligned (for 1.2), indeed.

Maybe this is generally something that we need to clarify with the community?

@jlaura
Copy link
Contributor Author

jlaura commented Dec 19, 2023

This PR originally had a plan to deprecate and migrate, but that was removed at some point while iterating. I tend to agree that having proj:epsg and proj:code in the same version does not seem to gain much and introduces more complexity (in validation, in creation, in tooling). For data creators/custodians that want to stick with proj:epgs, it seems they can just stick to versions < 2?

@m-mohr
Copy link
Contributor

m-mohr commented Dec 19, 2023

The only good reason why data providers want to stick with proj:epsg is likely backward compatibility. But if they stick with 1.x people will get back to tooling creators (ol-stac, stac-layer) for example and ask for proj:epsg support, but they likely want to get rid of it at some point. Overall I think we want as many people to migrate to proj:code, so the least painful way to me seems like allowing both for some time. That's just my point of view though.

I tend to agree that having proj:epsg and proj:code in the same version does not seem to gain much and introduces more complexity (in validation, in creation, in tooling).

It's actually the opposite, I think. It should make it less complex (except for the JSON Schema for 1.2).

@m-mohr m-mohr requested a review from philvarner March 8, 2024 11:54
@m-mohr
Copy link
Contributor

m-mohr commented Jul 11, 2024

I'm sorry that this is still not merged yet @jlaura. I think with the release of STAC 1.1 and the new bands contruct, it's a good time to finally release this so that people can make the breaking changes in one go.

@philvarner Have your concerns be adressed? If yes, please approve.

I'd propose to release a 1.2 with both proj:epsg and proj:code and directly after release 2.0 without proj:epsg. I don't think it will hurt anyone to have two versions, but may allow users to gracefully migrate. I'm happy to do the corresponding changes and releases.

@m-mohr
Copy link
Contributor

m-mohr commented Jul 22, 2024

The review request wasn't answered in the last 4 months and it seems we resolved it, so I'm going to override it.

@m-mohr m-mohr changed the base branch from main to v2 July 22, 2024 12:24
@m-mohr m-mohr changed the base branch from v2 to main July 22, 2024 12:25
@m-mohr m-mohr merged commit 46bbb33 into stac-extensions:main Jul 22, 2024
1 check passed
@jlaura
Copy link
Contributor Author

jlaura commented Jul 24, 2024

@m-mohr This is awesome! Thanks.

(Here's hoping I can be back on the calls once daylight savings or whatever happens again in October/November.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IAU
7 participants