Invalid `datetime` handling introduced by #82

[fmigneault]

@tik65536

Error seems to happen with #82

I'm trying the following request:
http://localhost:8888/dggs-api/collections/manitoba-rcm-ard/dggs/IGEO7/zones?parent-zone=021

Where manitoba-rcm-ard is temporal with datetime defined the usual way.

When reaching this point, the collection gets filtered-out, because ?datetime= was not provided.

pydggsapi/pydggsapi/routers/dggs_api.py

Lines 472 to 481 in 491b85a

	cp = collection_provider[v.collection_provider.providerId]
	# if the collection consists of datetime_col or timeStamp, then it is a temporal collection
	is_temporalcollection = (cp.datasources[v.collection_provider.datasource_id].datetime_col is not None or v.timestamp is not None)
	if (include_datetime and not is_temporalcollection):
	# skip non-temporal collections
	skip_collection.append(k)
	# For a non-temporal query, only filter out collections that have a native temporal dimension
	if (not include_datetime and cp.datasources[v.collection_provider.datasource_id].datetime_col is not None):
	# skip non-temporal collections
	skip_collection.append(k)

However, in this specific case, it should be allowed, since datetime does not really matter to list the zones.

(following using crim-ca/pydggsapi @ main)

Similarly, the following fails:
http://localhost:8888/dggs-api/collections/manitoba-rcm-ard/dggs/IGEO7/zones/021/data?zone-depth=0-2

Whereas it worked before (with the extra datetime dimension included):

In this case, ?datetime= could have an impact if there were other collections involved, but since it is on its own, it should not be filtered out.

I did not test when mixing collections with/without temporal, but I would expect it to merge the dimension accordingly rather than filter them.

[tik65536]

@fmigneault, Sorry, I forgot to handle the collections endpoint. I will remove the filtering from the zone query and the collections endpoints. However, I am currently busy with some other things, so I will probably fix it next week.

I did not test when mixing collections with/without temporal, but I would expect it to merge the dimension accordingly rather than filter them.

I don't understand the merge part. Are you referring only to the zone query and the collections endpoints? Not the zone data-retrieval?
In issue #65, where the zone-data retrieval query is split into temporal / non-temporal queries using the datetime parameter? So collections without a timestamp or without the datetime_col setting would not be considered in the query. So it won't merge the temporal dimension.

Tik.

[tik65536]

@fmigneault, I fixed the issues by skipping datetime handling for zone queries and zone data retrieval from the collections endpoint. I will go through a deep check later tomorrow before I merge the change and the PR #84

[fmigneault]

I don't understand the merge part. Are you referring only to the zone query and the collections endpoints? Not the zone data-retrieval?

"merge" as in padding the datetime with null for non-temporal collections that provided as single timestamp, as discussed in #65

It is fine to skip the other ones that do not have a datetime information since there is nothing to merge against.

No worries about the fix timeline.

[tik65536]

"merge" as in padding the datetime with null for non-temporal collections that provided as single timestamp, as discussed in #65

Yes, the merge between temporal and non-temporal collections with a timestamp does follow this logic.

[fmigneault]

Another case that I found, which involves "collection aggregation" (/dggs prefix):

http://localhost:8888/dggs-api/dggs/IGEO7/zones?parent-zone=021&zone-level=2&datetime=2025-04-01/..

If I simply remove the datetime query parameter:
http://localhost:8888/dggs-api/dggs/IGEO7/zones?parent-zone=021&zone-level=2

In that particular case, multiple collections should aggregate with null padding if possible. However, I just so happen to have no other IGEO7 collection on that API, so it is an "aggregation" of only itself. The error makes it look as if the zone refinement is inadequate, but it is only the default error message when no collection was matched. The "is temporal collection" check and omitted datetime places it in skip_collection, so no other are found.

Therefore, I can see a few options to adjust the behaviour :

1. If there are any temporal collection in the list, rather than skip_collection, it switches to "temporal-enabled" mode with the "merge" of temporal/non-temporal, regardless of datetime.
   1. If datetime is provided, it "reduces" the applicable range.
   2. Otherwise, it is the null-padded aggregation of all sorted datetime across all collections.
2. Alternatively, omitted datetime uses a sensible default (eg: utcnow()), and if it still doesn't match anything, a clear error indicates this detail so the user can adjust the request from the right cause.
3. Alternatively, returns an error indicating datetime is necessary directly.
   This might be relevant to avoid massive data requests, eg. if there are many collections with different datetime ranges causing a massive aggregate by default.

I see pros/cons in each case, so let's discuss the best approach.

[tik65536]

@fmigneault, I fixed the issues by skipping datetime handling for zone queries and zone data retrieval from the collections endpoint.

hey ~ @fmigneault . I don't get it; it won't happen, right? The temporal collection checking has been removed from the zone query endpoint (not yet merged to the main).

[fmigneault]

Probably. I didn't sync with all updates because of the other issues encountered.

[tik65536]

Hey, I have merged the changes into the main branch. I also checked the case mentioned above for the latest update. Since the separation of temporal/non-temporal collections has been removed from the zone query function, the second query will return the same result as the first.

Btw, thanks for your effort on those PR. Those are fantastic contributions, thank you!

[fmigneault]

I hope I didn't break anything on your end. Thanks for the validations!