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

Split subset and datetime/bbox #194

Open
m-mohr opened this issue Oct 17, 2024 · 21 comments
Open

Split subset and datetime/bbox #194

m-mohr opened this issue Oct 17, 2024 · 21 comments

Comments

@m-mohr
Copy link

m-mohr commented Oct 17, 2024

While it is very easy to support datetime and bbox (usually), it is much more work to actually implement subset with it A-Q(!) requirements. I'd propose to split subset and datetime/bbox into separate conformance classes to allow for an easy entry path.

@jerstlouis
Copy link
Member

jerstlouis commented Oct 17, 2024

Sorry I was confusing properties= with subset=.

Making them separate requirements classes would really defeat the purpose: the intent is that a client can always reliably use one or the other if subsetting is supported.

subset has a direct mapping to bbox and datetime for the spatial and temporal dimensions, plus it supports additional dimensions. If your coverages only have spatial and temporal dimensions, the only thing to do is the parsing of the parameter. It's a bit of work for implementing the server, but then it's done and clients can reliably use one or the other.

@m-mohr
Copy link
Author

m-mohr commented Oct 17, 2024

I don't understand why that would defeat the purpose (of what?)

Not sure whether it's better if I throw a 501 NotImplemented for subset just because someone randomly decided to put them in one conformance class.

@jerstlouis
Copy link
Member

Throw a 501 with a Not Implemented message until you have a chance to parse the subset= parameter and map it to your datetime and bbox implementation.

@m-mohr
Copy link
Author

m-mohr commented Oct 17, 2024

So throwing a 501 would be valid according to the spec? I actually don't plan to implement subset because for my case it doesn't offer anything on top of datetime/bbox (just having spatial and temporal dimensions).

@jerstlouis
Copy link
Member

So throwing a 501 would be valid according to the spec

It would not pass the compliance tests, but hopefully you will be bored one afternoon between now and the time when the ETS is ready to test your implementation, and you'll be able to implement the plumbing to re-reroute the subset request to your bbox / datetime implementation ;)

For the Testbed 20 TIEs, we can make sure the clients use whichever method you have implemented, if you don't have time or want to focus on other things before implementing subset.

subset is an alternative to datetime, bbox for spatial and time. subset is inherited from WCS so we keep it for the standard to be familiar for that community.

@m-mohr
Copy link
Author

m-mohr commented Oct 17, 2024

I don't say it should be removed, I just ask that there are two conformance classes, one for subset, one for datetime+bbox.
It's pretty much just a waste of time to implement the complex subset if you don't have other dimensions. So why require it? Didn't find a reason for it yet...

@jerstlouis
Copy link
Member

jerstlouis commented Oct 17, 2024

@m-mohr As I explained, so that clients can reliably use one or the other.

subset is the original parameter from WCS.

datetime and bbox are convenience parameters added in OGC API - Coverages.

A separate conformance class should provide different capability.

If the convenience is only available in some implementations, it's not a convenience, it's a hindrance.

@m-mohr
Copy link
Author

m-mohr commented Oct 17, 2024

100% disagree.

Why is it less reliable if they are split into two conformance classes? You can determine by the conformance classes which are available and can be used.

It rather feels LESS reliable if they are in one conformance class if that leads to 501's or implementation that don't report the conformance class because they only implement parts of it.

A separate conformance class should provide different capability.

Can you point me to where this OGC policy is written? I don't think this is a thing as it's not followed by various specs, e.g. OGC API - Processes.

@cportele
Copy link
Member

As an aside, 501 is almost never an appropriate response. According to HTTP Semantics:

[501] is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource.

So, if the server returns 501 on a GET request, the server states that it does not recognize the GET method for any resource.

@cnreediii
Copy link
Contributor

Not sure this helps. My take is that good practice (and the ModSpec) suggest that each requirements class and associated conformance test class have a constrained set of related and testable properties - such as one requirements class for specifying a horizontal CRS and one requirements class for specifying a vertical CRS. Mixing those two together into one requirements class causes confusion and probably many test failures. Many implementations only care about horizontal CRS and not vertical CRS. Hence, better to separate.

The ModSpec does provide requirements and guidance. For example, Req5: All conformance tests in a single conformance test class in a conformant standard SHALL have the same standardization target. And Req1: All the parts of a requirement, a requirements module, or requirements class SHALL be testable. Failure to pass any part of any requirement shall be a failure to pass the associated conformance test class.

And various Requirements state: A standard may have mandatory and optional requirements classes. This allows the options in the testing procedure to be grouped into non-varying mandatory and optional conformance classes. Each requirement within an optional requirements class is mandatory when that requirements class is implemented. When needed, a particular requirements class may contain only a single requirement.

@m-mohr
Copy link
Author

m-mohr commented Oct 17, 2024

@cnreediii If you apply that to this issue, what would that mean (in your opinion)? I'm not sure I can really follow and conclude on something based on your message...

@cnreediii
Copy link
Contributor

@m-mohr I am saying "I'd propose to split subset and datetime/bbox" I would also say any requirement that has A-Q "sub" requirements is waaay too long and complex!

@jerstlouis
Copy link
Member

jerstlouis commented Oct 17, 2024

Why is it less reliable if they are split into two conformance classes? You can determine by the conformance classes which are available and can be used.

Clients can choose to use only one of subset or datetime/bbox if both are supported.
Having to use a different parameter based on the supported requirements class would mean that clients wanting to be interoperable with any implementation supporting subsetting one way or the other would need to implement support for both.

So that would be forcing the client instead to implement both, complicating the development of clients at the cost of making it a bit easier for implementing subsetting on the server side.
There tends to be a greater variety of clients than servers, hence the decision to require server implementations to implement both approaches instead.

501's or implementation that don't report the conformance class because they only implement parts of it.

Implementations should support both, so a conforming implementation should not be returning a 501.

As an aside, 501 is almost never an appropriate response.

I am understanding this as @cportele saying it would be better to return a 400 for an unsupported parameter until it is implemented.

Can you point me to where this OGC policy is written? I don't think this is a thing as it's not followed by various specs, e.g. OGC API - Processes.

I don't think that's an OGC policy, but it is a design principle which makes a lot of sense to me, for the reasons stated above.
The guidance from the ModSpec does not seem to address how to map functionality to requirement classes, beyond what @cnreediii mentioned above. In my opinion, there should be guidance in the ModSpec that a requirements class gathers a particular set of functionality. An alternative way to do the same things for convenience should not be split out. Similarly, multiple mandatory requirement classes should also be grouped together in a "Core".

The same subset + datetime, bbox organization is in OGC API - Tiles and OGC API - Maps:
This follows the exact same pattern.

subset=time() and datetime support in OGC API - Tiles:

http://www.opengis.net/spec/ogcapi-tiles-1/1.0/req/datetime

subset=time() and datetime support in OGC API - Maps:

https://www.opengis.net/spec/ogcapi-maps-1/1.0/req/datetime

Spatial subsetting (bbox and subset=Lat(),Lon() / subset=E(),N()) in OGC API - Maps:

https://www.opengis.net/spec/ogcapi-maps-1/1.0/req/spatial-subsetting

There is no spatial subsetting in Tiles, since the 2DTMS row/column already takes care of that.

There is also general subsetting in Maps:

https://www.opengis.net/spec/ogcapi-maps-1/1.0/req/general-subsetting

which is through the subset parameter only. General subsetting is not specified in OGC API - Tiles, but it can also be supported and is specified for Map Tiles in OGC API - Maps, and Coverages Tiles in OGC API - Coverages.

I am still very strongly opposed to the idea of splitting the convenience parameter datetime from subset=time, or the convenience parameter bbox from the corresponding subset=Lat(), Lon() / subset=E(),N(), because they ought to be together to be a convenience, and that is the approach in Maps & Tiles with the explicit intent that it would be consistent with Coverages. If the convenience is optional or an alternative, it is no longer a convenience for the client.

Regarding @cnreediii 's comment about:

one requirements class for specifying a horizontal CRS and one requirements class for specifying a vertical CRS

There are actually not separate CRS in the spatial subsetting requirement classes, perhaps with the exception of EDR.

bbox supports 4 or 6 coordinates and we have 3D CRSs like CRS84h and EPSG:4979.

Regarding the extra atmospheric pressure levels, after much discussion (e.g., opengeospatial/NamingAuthority#228 ) the decision was that this is completely separate from the CRS and fits within the general subsetting mechanism.

I would personally have less of an issue with splitting the subsetting requirements class into "temporal subsetting", "spatial subsetting" and "general subsetting" (than splitting the convenience parameters), as it is in OGC API - Maps. I'm not sure whether others in the Coverages community would be happy with this though, since in general there is a strong desire to keep the nature of coverage dimensions arbitrary.

I would also say any requirement that has A-Q "sub" requirements is waaay too long and complex!

This split would help with that. As an implementer, I personally find it much easier to have all related aspects of one parameter within the same requirements (to the extent that this is practical), than having to look through multiple requirements to see whether it is related to the same parameter. A-Q are all about the subset parameter. I do agree it's a lot of letters :)

@m-mohr
Copy link
Author

m-mohr commented Oct 18, 2024

Well, then my conclusion from this is that there shouldn't be two options for simplicity but just one, I.e. remove datetime/bbox. If you expect any clients, no convenience options are required to. The same applies for scaling where se options pretty much do the same. My general impression ist that Coverages Core is more difficult than it needs to be (which somewhat disqualifies it from being the basis for GDC).

@jerstlouis
Copy link
Member

jerstlouis commented Oct 18, 2024

@m-mohr The reason datetime and bbox are there is for the convenience of clients / users familiar with these parameters, for examples those familiar with WFS, WMS, and OGC API - Features.

In Maps and Tiles, we did the opposite, add subset in additional to the traditional bbox in WMS, so that it feels familiar to those coming from WCS, and because subset avoids any potential order confusion (plus it supports additional dimensions).

It means a little extra work on the server side, allowing clients to pick their favorite way. It's really not that difficult to parse both parameters. We've discussed all this over a period of 4+ years (for example see #152), and as was brought up at the meeting on Wednesday, we have really been hoping to achieve a stable API rather than going back over decisions taken a long time ago when we had a stronger regular attendance to SWG meetings, and pulling the API in different directions. The consistency with the other OGC APIs (Maps, Tiles) should also be considered.

@m-mohr
Copy link
Author

m-mohr commented Oct 18, 2024

Okay fair with regards to the removal. I don't see any discussion on splitting into two conformance classes in that issue though.

@jerstlouis
Copy link
Member

@m-mohr I meant we had lengthy discussions about whether to add bbox in addition to subset before adding it prior to that issue in 2021. Some of that discussion might have been in SWG meetings and not in issues, but see for example #58 and #84 from 2020, and 10d7312 from December 23, 2019 when the bbox parameter was added.

e.g., #58 (comment) from Feb 2020

Note, I'd propose this as an additional convenience syntax, as I can se the utility in also being able to subset arrays not expressible by bbox or datetime

@m-mohr
Copy link
Author

m-mohr commented Oct 18, 2024

I don't really see any discussions about splitting into two conformance classes though. Anyway, I'll shup up now,, seems to be a waste of time to get involved here.

@jratike80
Copy link

Entertaining discussion and good points from all parties. Not waste of time at all.

For my mind what is best in WCS 2.0 is just explicit subsets subset=Lat(-90:90). No need to think about the CRS and the order of the axis. Subset is convenient by natur.
Maybe I support @m-mohr, perhaps bbox+time could be removed. On the other hand, I do understand the convenience of bbox for users who have already a bbox for downloading features from OGC API Features and who want to get some coverage data from the same area. Pity that OGC API Features does not support subsets.

@jerstlouis
Copy link
Member

Thanks for chiming in @jratike80.

Maps does support both, as do Zone Queries in OGC API - DGGS.
OGC API - Tiles as well for dimensions other than the 2D of the 2DTMS.

@jerstlouis
Copy link
Member

jerstlouis commented Nov 6, 2024

We discussed this issue in the OGC API track Coverages SWG session in Goyang on November 6, 2024 and there was a general agreement that we primarilty want to make life easier for the clients.

Some informative guidance as to how to map the different convenient parameters to more general ones should also be provided to make it easier for servers to support them all.

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

No branches or pull requests

5 participants