-
Notifications
You must be signed in to change notification settings - Fork 85
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
Clarification on multiple bounding boxes in an extent #520
Conversation
In STAC we allowed multiple extents being used in 0.9.0 (see PR radiantearth/stac-spec#856). This got also inherited into the openEO specification then, which would also need to change. STAC allowed it for a use-case by @davidraleigh so there's at least one server-side implementation, but I haven't seen many more so far. On the client-side at least STAC Browser and openEO tooling implement that the extents are all shown on a map. The clarification would lead to a strange visualization with the overall bounding box overlapping the "sub bounding boxes". Not ideal, but I guess that is also not a deal-breaker. |
Thanks very much @m-mohr for the research and the feedback on the current use of the extents, very helpful. If there is any feedback from the STAC/openEO community, if such a change would be helpful guidance for clients going forward, that would be of interest, too. If that is not the case, I would say that the SWG should look at this again and see, if we can come up with a solution that addresses the concerns from both STAC and OGC API Common. |
Not yet, we still have to discuss the issue. I hopefully have some more details about it End of March (after vacation). |
@cportele - sorry for the slow response. We'll huddle with the STAC community. My take is that we're at a time where it's possible for us to entertain a change, as we are just before 1.0.0, but we've been trying to be not rock the boat much at all during the release candidate process. But this does feel like a pretty minor clarification, and one that seems beneficial. Post 1.0 I think we're locked in and I don't think it'd be good to change, as our Collection is based on the OGC Collection, and want them to stay in lock-step and wouldn't want a behavior change. But @m-mohr is our main implementor for affected tools, working on both OpenEO and STAC Browser, so I defer to him. I can try to reach out and check with David R. |
This is an observation that is not directly related to the question of multiple bounding boxes, but associated to the comment above. In the example given above by @cportele there seems to be a typo in the westBoundLongitude of the main bbox which, assuming the values for the three component boxes are correct, I think should be But it begs the question as to how do you construct an embracing box around multiple boxes. When the antimeridian is crossed, just taking the minimum and maximum longitude values is insufficient. ISO 19115 requires bbox longitudes to be in the range -180 <= longitude <= +180 degrees. There is a general question of treatment of boxes that cross the antimeridian. (This may be a separate issue) |
@RogerLott AFAIK west/east is always the definition, not min/max: https://github.com/opengeospatial/ogcapi-features/blob/master/core/standard/clause_7_core.adoc#user-content-parameter-bbox |
@rcoup - for bounding box, yes, east/west. ISO 19115-1 uses westBoundLongitude, eastBoundLongitude, southBoundLatitude and northBoundLatitude; API - Features shortens these to westBoundLon, eastBoundLon, southBoundLat and northBoundLat. (I am not sure that ave two different strings helps standardization, but that is a seperate issue). |
@RogerLott - sorry, I missed your original comment, but thanks to the discussion I have now seen it.
I don't think so. 172.461667 is west of -178.334698.
If the component boxes do not have overlapping longitude values, there can be multiple ways how to define the embracing box. If I have a dataset with data in Japan, Germany and Chile, which one do I use:
All options are a single bbox that contains the three country bboxes. Do we want to prescribe which one must be used? Which one, the one with the smallest area? I think we should leave that decision to the provider. In the US example there is no option as the three boxes have overlapping longitudes. The anti-meridian is crossed, if the west-bound longitude is larger than the east-bound longitude. This is stated in the text above example 5. Or am I missing something? |
@RogerLott - Where are these tokens used in OGC API - Features? I don't remember them. |
@m-mohr - Thanks for the response 👍 . Could you clarify the concern with respect to the antimeridian? It seems that I am ignoring something, but I do not understand what the issue is. |
@cportele said "172.461667 is west of -178.334698". Are you sure about that? -178 seems to me to be less than +172. "In the US example there is no option as the three boxes have overlapping longitudes". I would suggest that in the US example the fact that the individual boxes ovrrlap is irrelevant. If you are describing the 50 states of the US then your north-up box needs to be centred on the US regardless of longitude, and then you take the left edge of the leftmost box as west and the right edge of the righmost box as east. To me that seems the only logical option - because you are describing one entity having three parts. But take Russia including the enclave of Kaliningrad. One entity, two boxes. Kaliningrad longitude is approximately +19.2 to +23.4. Russia proper from +26.5 through +179.9 across the antimeridian to -169. The minimum is -169, maximum +26.5. To describe a bounding box for one entity, Russia, as from -169 in the west across Canada, Greenland and the Atlantic to 26.5 in the east would (in my view) be perverse, and more importantly it would fail to meet the API Common requirement to includes a coordinate that is part of the (spatial) geometry of the resource. Russia's extent is from +19.2 in the west across Siberia to -169 in the east. Three boxes related only by being in the same dataset - Chile, Germany, Japan - does not have the same logic of there being one entity with multiple parts. Now we have three separate entities. It does not really matter how your overall box is described, but if you are using 19115-1's bounding box with its prescribed longitude range of -180 to +180 then logic suggests you take the western longitude of the western entity (Chile) as west and the eastern longitude of the eastern entity (Japan) as east. This does not cross the antimeridian and there is no issue. So I suggest that the logic is to put a north-up box around a dataset with the norhern edge centred on the dataset. then find the left-most and right-most longitudes from this. The critical issue is to be dataset-specific in constructing this box, in the first instance ignoring longitudes, then get the longitude values. I don't think you can correctly construct an overall box simply by examining the values of component boxes. |
uh huh, but "smaller number" is different from "west", and "larger number" is different from "east". It's a sphere, you can head either east or west to get from one point to another... so we can't use minimum and maximum longitude values (except possibly as a tie-breaker). Minimum area is probably the most deterministic approach to calculate it IMO. For your Russia example:
Leading to bbox options:
Recommend but not prescribe the smallest area approach? |
That would work for me. I will wait a bit to see, if there are objections or concerns to this. If not, I will add this to the PR. I am beginning to think that we may be talking about different things.
I do not understand what "entity" means in this context. This pull request is about the spatial extent of a feature collection, where the current requirement is that each feature must be inside the bounding boxes describing the spatial extent. In the US example the features in the collection could be the states or any other set of features that are located in the US. The Chile/Germany/Japan example was not about three features but about some hypothetical collection of features where all features are located in one of the three countries. A bounding box in WGS 84 longitude/latitude consists of four numbers, the west-bound longitude, the south-bound latitude, the east-bound longitude and the north-bound latitude. In most cases the sequence is minimum longitude, minimum latitude, maximum longitude and maximum latitude, but when the box spans the antimeridian the first number (west) is larger than the third number (east), since longitudes are in the [-180, 180] range. All of this is already documented in the current standard. The clarification that this pull request adds in
The details how the component boxes or the overall box are generated is up to the provider (if configured manually) or the software tool (if derived automatically). As long as the requirement above is met, it is a valid bounding box. We can discuss adding recommendations like the one discussed with @rcoup above, if there is consensus that this adds value, but I currently do not see that there is value in adding additional requirements how to construct the bounding box array. |
Meeting 2021-04-12:
|
@cportele We just had the STAC call and I can report that we are fine to introduce the proposed change into the STAC specification. We still need to find a concise way to describe it without copying all the requirements directly though. |
Awesome, thanks @m-mohr. I will also have a look, if I see a way to improve the wording. |
@cportele I'm not sure it needs improvements to wording (that was not my point), it just better fits the STAC specification if we can summarize the requirements in one or two sentences (having in mind extents are required in STAC and also require at least one array element, which makes the overall specification easier to write for STAC). Either you can propose something or we'll come up with something and ask you to verify we captured it correctly. |
👍 I will give it a try. |
@cportele I like the description you added. I'll use the following part in STAC and openEO then.
|
@cportele I'm looking at the https://github.com/opengeospatial/ogcapi-features/blob/master/core/openapi/schemas/extent.yaml right now and I think the OpenAPI spec should also reflect the changes. Here's a proposal (I'll add it like this to openEO and STAC API):
Note: It also moves "The value |
Thanks for the proposal, @m-mohr !
👍 Thanks @m-mohr. That makes sense, I have added this as another commit. |
@cportele Should we go ahead and merge the STAC PRs (i.e. is this PR surely to be merged) or should we wait (timeline?)? |
@m-mohr - The plan is to have a motion to merge this PR in the next meeting of the SWG, which is in April 26. If you can wait, I would suggest to wait until then, or does this create an issue for STAC? Thanks! |
@cportele @m-mohr for whatever it is worth, I looked over the proposed changes and they look good to me; I like this PR. I would not be opposed to merging ahead of time to accommodate STAC's timeline. If anything comes up in the April 26th meeting we can create another issue and perhaps another PR to address. |
@cportele @pvretano Thanks for the details. We just merged it to the STAC spec and will release it as RC3 probably on Tuesday and we just hope the best that it gets through. If it doesn't get merged, then we'll just issue another RC, I guess. So don't worry, it's not required from our side to do anything before 26th of April. |
Meeting 2021-04-26: SWG agrees to merge the PR after the change. @cportele will continue the discussion with @RogerLott and if any additional issue is determined we will create a new issue as a result. |
789e90f Merge pull request #1139 from radiantearth/dev 76d1560 Merge pull request #1140 from radiantearth/stability-note-update 1995967 removed stability note and moved key info for 1.0.0 e5ec604 Merge pull request #1135 from radiantearth/versions-to-1_0_0 ca90fa4 Merge branch 'dev' into versions-to-1_0_0 5d017a4 Merge pull request #1138 from radiantearth/itemcollection-heuristic c40ced1 Merge pull request #1137 from radiantearth/python_ci_validation 036f291 CHANGELOG entry added 2e60c61 Remove ci job from workflow 1b2fefb Removed ItemCollection from STAC detection heuristic, may get invalid soon. radiantearth#141 9badaa7 Comment out python ci validation d584c28 updated versions and changelog to 1.0.0 517c5fb Merge pull request #1134 from radiantearth/updated_uml 59434f3 updated UML allowing collection children in catalogs ef679f4 update UML to remove summaries from catalogs c197acb Merge pull request #1133 from radiantearth/title-rec 45d0789 Merge pull request #1131 from jbants/dev 24ec4a2 added best practice to include title, updated examples 8b14d60 Added clarity around CI tests d811ae2 Ci validation (radiantearth#2) 4ecbabc Merge pull request #1128 from radiantearth/dev 7363844 Merge pull request #1129 from radiantearth/changelog-fixes 8f35589 minor changelog fixes 09f0019 Merge pull request #1119 from radiantearth/version-rc4 21e12d5 Updated CHANGELOG e7d008f Merge remote-tracking branch 'origin/dev' into version-rc4 00243b3 Update date in changelog f45da2c Merge remote-tracking branch 'origin/dev' into version-rc4 a553f11 Merge pull request #1127 from radiantearth/issue-1125 ba9df2e Update collection-spec.md b43d16b Merge pull request #1123 from lossyrob/clarification/rde/collection-extension 90a2cbd line break for linting bf02447 updates from PR review 0a6da1a Collection: Open intervals for temporal extents #1125 c9323e4 line break for linting b24066e update from PR review ea3ee26 reorg and edits 59a8cf4 Update CHANGELOG cd08d4a Fix TOC ordering in extensions d87813d Clarify when to include extensions in stac_extensions 507a813 update version numbers bb7cb80 merged dev 7be086c Merge pull request #1122 from radiantearth/catalog-summaries-removal 9fa29ce Update catalog-spec/json-schema/catalog.json 9fb18e7 Merge pull request #1121 from radiantearth/remove-core-ext-schema 42753fc Catalogs don't support summaries any more 7406ed7 Merge remote-tracking branch 'origin/dev' into remove-core-ext-schema 50d12f6 JSON Schemas don't allow "shortcuts" for core extensions any longer 56205ea updated version numbers / changelog 08fe31d Merge pull request #1116 from jjrom/dev 4b77ef8 updated changelog 3f9d9ac Merge pull request #1114 from radiantearth/emmanuelmathot-patch-1 34a7c0e fixed schema to reflect intent, update language 2f81fcc Allow empty catalogs 5c48425 add missing projection declaration f81324f Merge pull request #1113 from radiantearth/dev 2f0a35a Merge pull request #1112 from radiantearth/version-changes-rc3 094b90d fixed linking issue d6103b8 update versions, changelog edits b398536 Merge pull request #1111 from radiantearth/foundations-overview 06aea1c less characters in a line dfeaf64 more fixes from PR feedback 17c1493 updates from PR review 3a3cc09 updated changelog with pr link 20f7e71 new overview section 311a9a3 Merge pull request #1110 from radiantearth/bp-clarification 431e0af Update best-practices.md c5d96a8 Merge pull request #1109 from radiantearth/catalog-json-schema c397767 best practices: some clarifications #1108 9abb6e6 Catalog: Require child / item in JSON Schema #1100 89923bf Merge pull request #1107 from fredliporace/summaries 13bec82 Update collection-spec/collection-spec.md 6181a35 Merge branch 'dev' into summaries 31e9f91 Apply suggestions from code review 5b5a8bd Merge pull request #1093 from radiantearth/json-schema-summaries a6f838a Merge pull request #1101 from radiantearth/root-parent 3e3677c fix from PR review e5060dc Update collection-spec/collection-spec.md 4e4a7f7 Filled out 3rd option for summaries d6062ff Merge pull request #1105 from radiantearth/gsd-tweaks 91a435e Merge pull request #1104 from radiantearth/no-scientific-1093 7762cca Fixing .md link 16b4b42 Changelog for issue #1106 457a5d5 Rephrase summaries; exception for summary schema; typo 476e179 Update CHANGELOG.md ab58068 Update CHANGELOG.md 6fb42d9 update changelog 5890f7e clarifications on using gsd as an asset field 7d3a94a Merge branch 'dev' into json-schema-summaries 0d8916f remove scientific extension ee2ca2e Merge pull request #1102 from bradh/common_metadata_2021-04-25 ea23b29 minor editorial cleanup for common-metadata 314cfa1 include parent and root rel types, even for same file d42e3c1 Merge pull request #1092 from radiantearth/preview-rel-type 59ad837 Merge branch 'dev' into preview-rel-type b8d8b5e Merge pull request #1099 from radiantearth/utc 14fec94 More emphasis on UTC requirement 1f3b72b All timestamps in UTC #1095 dfb2300 Merge pull request #1091 from radiantearth/clarify-root b74d5b8 Better integrate example e257928 Couple of changes for better alignment between the specs + list formatting 54d7be4 updates from review comments b388c25 Merge branch 'dev' into clarify-root 091af68 Merge pull request #1097 from bradh/patch-1 59df72d Some more details 233b757 Minor change to Collection usage wording eb7b9d2 Attempt to provide the main text ee8022b Example (WIP), more details 40ab954 Update collection-spec/collection-spec.md 6755d41 First try at supporting JSON Schema in summaries #1045 6b24f7c put note up top 60bf0bc more updates 5a994b7 Update overview.md 04664cb Add preview relation type #1090 2be5e04 got rid of old changes that snuck in d0336de updates to make clear root can be catalog or collection bb15ca6 Merge pull request #1080 from radiantearth/fix_examples 16e80e9 Update examples/collection.json 1f9c4b0 Merge pull request #1086 from radiantearth/markdown 2daab18 Fix CI 77577a1 Merge branch 'dev' into fix_examples 2142d4b Merge pull request #1078 from radiantearth/rephrase-stac-extensions 6405e0d Merge branch 'dev' into fix_examples 9b438b6 Merge branch 'dev' into rephrase-stac-extensions 9f866c0 Merge branch 'dev' into markdown 40dfb8b Merge pull request #1085 from radiantearth/extent-clarification 6a5a153 Merge pull request #1087 from radiantearth/validate 93765a5 Made requirement for collection fields / rels clearer 82d3a22 Merge pull request #1089 from radiantearth/fix-collectionless-item 840a4cf Remove collection field 9c6be4b Collectionless item has collection field & link #1081 6c8df30 Item: Validate that collection property is set if link with collection rel type is available #1088 90f8d37 Update STAC Node Validator and fix examples 6f57298 Fix Markdown be40027 Clarification on multiple bounding boxes in an extent. #1064 opengeospatial/ogcapi-features#520 6b1bd4c fix complete 8617240 Update catalog-spec/catalog-spec.md 21ecb16 Update collection-spec/collection-spec.md 53686bd simple item has no title 50d4862 stac_extensions for summarized fields 1ad7d1b updated changelog a8354e1 datetime extent in collection 0684c3a align spatio-temporal extents bcb5d96 removed duplicate the d73b43f Clarified that stac_extensions should also list extensions that are used in Collection summaries git-subtree-dir: stac-spec git-subtree-split: 789e90f
See #518 for the background of this pull request.
The spatial and temporal extents support multiple bounding boxes (
bbox
array) and time intervals (interval
array). This was intended as an extension point for the future and Features Core recommends to use only one item in both arrays.This pull request adds a new requirement to clarify the content of these arrays in cases with multiple bboxes/intervals: the first bbox/interval would always describe the complete spatial/temporal extent and the other ones will be the more precise extent description. The goal is to support both clients only interested in a single bbox/interval and those that can handle more.
An example: The following extent represents the United States of America (excluding Territories). The first bounding box of the four bounding boxes is the union of the three other bounding boxes representing the 48 contiguous states, Alaska and Hawaii respectively. Note that the overall bounding box as well as the bounding box of Alaska crosses the anti-meridian.
This change is intended as a clarification. While this pull request adds a new requirement, the proposed change should not break any existing implementation or deployment, because currently all deployments that we were aware of in the SWG discussion on 2021-03-15 are all using a single bbox / interval only. I looked at a number of deployments and did not find one that uses more than one item in the arrays.
If that is the case, which we need to verify, this is probably the time to make the change since the clarified wording supports both clients that only want to overall extent and those that can handle extents with multiple areas / time intervals, while the current wording may lead to interoperability issues as described in #518.
If it turns out that there are already deployments that are not consistent with the proposed requirement, we should keep the clarification, but downgrade it to a recommendation.
The STAC Collection spec would also need to be updated with the clarification.