-
Notifications
You must be signed in to change notification settings - Fork 17
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
Arrays and objects in eq and neq processes #208
Comments
Thank you for the feedback. I understand that this can be confusing or misleading. I will discuss this with our team, but it can be problematic to change this behavior as it would be a breaking change. |
Why is that I agree that it would be better to allow a backend to throw some kind of error when it doesn't want to compare possibly nested structures. Note that this would only happen when both operands are array/object. You can easily do |
We didn't want to dig into describing how arrays and objects have to be compared and traversed, yes. That's a rather complex topic and seems to be handled pretty differently across environments and as such the desire was to keep the process simple by just disallowing non-scalar values but at the same time not interrupting with errors if something "unexpected" occurs. I mean I would not expect an exception from an |
From a user's perspective, I'd expect a missing value as a result in that case, for both processes. |
From a programmer perspective, I'd find anything else than boolean values confusing. That's probably what influenced the current situation most. While I think |
It's not the
That I would prefer that it is optional for a backend to implement array/object traversal for comparison and just throws an error when traversal is too hard. |
also note that inconsistent definition of equality operators (on arrays/objects) will be annoying for testing purposes like in #204 |
Interesting points, @soxofaan. I wasn't aware Python can actually throw errors for comparisons. I haven't seen that before. So then I guess the cleanest solution would be to go with an exception. That is a breaking change though and as such needs to go through PSC vote. Do we think we have enough good reasons for that? Would anybody be up to write the PSC proposal?
But then of course you need to define in the process how that should be done, right? It's not a good idea to leave it up to implementation and then get different results across back-ends depending on how they prefer to compare. Just for reference, here's some cases one would need to think about in JS: https://github.com/denysdovhan/wtfjs#array-equality-is-a-monster ;-) |
Not from the R programmer perspective: > 5 == list("a", "b")
[1] NA NA
Warning messages:
1: NAs introduced by coercion
2: NAs introduced by coercion |
So many insights into different programming languages at the moment, wow. So we have the issue that different programming languages do different things, and I'm obviously influenced by other languages than R and Python. So now the question is, what is the basis we want to go for in openEO? We can't suit all languages, it will be either exceptions, null (NA) or boolean. |
it is well know that equality checking in JS is not for the faint of heart 😄
about the
The advantage of exceptions is that it more clearly communicates that it is a backend specific "limitation", and it allows a backend to improve their implementation incrementally without changing/breaking the working part of the API. |
Open to discussion, but there are two things we need to decide on:
And it gets complicated. That's why it has not been considered yet, especially as it makes writing a test suite #204 much harder.
Indeed, if we go the complex route, exceptions need to handle it, otherwise results are not comparable. But: I think we should not make this process overly complex and just define array_equals if that is something that users need. And object support is very limited at the moment anyway (we can't even get a value from an object by key), so I don't see why we should allow comparing it deeply. |
I agree that a backend's implementation can be complicated (although I would not overestimate this), but that does not mean that the process spec would be complicated. If we limit the scope to just arrays (support for objects is indeed not that useful at the moment), it's not that hard to describe how array equality (and inequality) is defined: arrays have same length and all pairwise corresponding array items are equal.
I wonder if defining a dedicated I just think that the abstraction level of openeo as a programming language allows to work with a single |
The advantage of separate processes is that you can easily detect whether something is supported or not. The other option would be to just remove array/object from the list of supported types in the parameter schemas. I guess I'd be okay to implement it for arrays and define exceptions for objects, but this would need to go through PSC anyway. |
Proposal: Restrict the schema to only allow scalar values. |
Both
eq
andneq
processes have the following expected behaviour:Wouldn't it make more sense to raise an exception in this case? I find it counterintuitive that the two processes, which are supposed to be complementary, return the same result.
The text was updated successfully, but these errors were encountered: