count: `condition=false`?

[soxofaan]

https://github.com/Open-EO/openeo-processes/blob/d0ce91fcd347360b907ea2d9589d7564a2c1e1e3/count.json#L56-L64

count supports, apart from a full process graph, two special values for condition:

• null as shortcut for is_valid
• true as shortcut for count all

The schema of condition in the docs at e.g. https://processes.openeo.org/#count say

process-graph:object|boolean|null

Which also include the boolean value false, but its is undefined in the docs.

possible definitions:

• always count 0 (which is conceptually the cleanest I think, given the behavior of true)
• count invalid values (feels very adhoc, but would be a useful shortcut I think)

[soxofaan]

background story:

• this came up when doing support for https://discuss.eodc.eu/t/reduce-dimension-using-count/420
• under https://github.com/Open-EO/openeo-python-client/issues/317 , before I realized this was undefined, I actually added unit tests with condition=false, e.g. https://github.com/Open-EO/openeo-python-client/blob/d608d44f36954540f208a9e699cfc1b2e7a7fad1/tests/rest/datacube/test_processbuilder.py#L687

[m-mohr]

false is not allowed as input value according to the spec. The part that is defining that is "const: true" in the boolean schema. That means false must be rejected right now (comparable to an integer value with constraint "minimum: 0", which would also reject all values < 0).

Honestly, I regret defining the condition parameter as it is right now. It should have been defined in at least two processes:

• count_valid(data) (formerly: condition = null)
• count(data, condition = null) (formerly: condition = true / process)

That would have been more straightforward. Or I should have used string values instead of the null/false "mis-use". It's just not very intuitive.

[soxofaan]

false is not allowed as input value according to the spec. The part that is defining that is "const: true" in the boolean schema.

Indeed, I see that from the spec in JSON, but the docs are misleading, because they say "boolean" in general

And apart from that it is very confusing on itself (as you note yourself), that value true is allowed, but false is not

[m-mohr]

Yes, but that is normal behavior in programming language signatures?! It just lists the types/classes without value constraints, otherwise you'd also need to list all other constraints in the signature, e.g. min and max values for numbers or allowed string values for strings.

[soxofaan]

I understand, it's just very weird to see "boolean" as type, but false is illegal.

Also note that the representation of these union schema's (I don't know the correct terminology) in the docs is pretty obscure. Because I've been already exposed to a bit of JSON schema, I kind of get it, but I wonder what the less-initiated user makes from this visual structure:

Anyway, I think we should add an explicit note in the docs that false is (currently) not supported

[m-mohr]

Also note that the representation of these union schema's (I don't know the correct terminology) in the docs is pretty obscure.

Yes, I agree. Please open an issue in openeo-vue-components with suggestions and we can try to improve it.

[m-mohr]

ToDo:

• [x] Explicitly mention that false is not allowed.

[soxofaan]

Please open an issue in openeo-vue-components with suggestions and we can try to improve it.

Is that something we have much control over? I thought this just part of the doc building tool

[m-mohr]

That is something we have control over, the doc building tool is based on the openEO vue components and is developed by me :-)

[soxofaan]

:laughing: I didn't know that, I assumed we were using an off-the-shelf OpenAPI doc generation tool

[m-mohr]

Yes, we do. For the API. But our processes have nothing to do with OpenAPI. :-) The processes are something custom in mainly JSON Schema. The DocGen for the Processes ist just a slightly extended version of the Process(es) Vue Component that we also use in the Web Editor etc.