search

Open-EO / openeo-web-editor

An interactive and easy to use web-based editor for the OpenEO API.
https://editor.openeo.org
Apache License 2.0
12 stars 17 forks source link

comparison processes read 4-digit values as EPSG, change them wrongfully #157

Closed jonathom closed 3 years ago

jonathom commented 3 years ago

Consider the following to reproduce: Web Editor -> open a process lt, gte or the likes, at y parameter select type number and enter 4000, click save, open the process again, now type is EPSG and the first in the selection list is EPSG:2000, click save, now the value for y is 2000. As far as I can tell only 4 digit values and processes lt, lte, gte and gt are affected, didn't test all combinations though, i.e. only had a look at add and divide to confirm this is not affecting other math processes. (All while connected to VITO backend).

m-mohr commented 3 years ago

Yeah, the issue that another data type is "chosen" is not really possible to fix. It also applies to other data types. The issue is that just from the value (e.g. 4000) I can't detect which "visualization" to choose. I should probably fall back to the "native" one (e.g. number instead of EPSG) by default, but it will never be "exact". Then there will be cases where an EPSG code is identified just as a number...

What I can surely fix that EPSG:4000 is chosen (if it exists) instead of 2000...

Related: #80, #43

jonathom commented 3 years ago

Why allow EPSGs at all?

m-mohr commented 3 years ago

Any data type is allowed so why restrict? I can't guess what the user wants to input...

jonathom commented 3 years ago

True. Why is any data type allowed, given the documentation:

If any operand is not a number or temporal string (date, time or date-time), the process returns false.

? I don't wanna pry too much here, I just fail to imagine any use case.

Here's an idea: The menu gets a neutral option like "-Select type-". Whenever process options are opened, this "-Select type-" is shown in type field instead of EPSG or a best guess type. The value is represented from process graph, there is basic typing possible here I think (like show text field / bounding box depending on input). And when the value is changed it is just written to process graph as it was there before, if type is still set to that neutral option. This whole thing possibly needs some documentation in the process box then.

m-mohr commented 3 years ago

Okay, there were two issues here:

  1. EPSG selection had a bug that always made the select box reset to the first entry (2000) - this has been fixed.
  2. The detected data type was just taking the one that was alphabetically first if a conflict was detected. EPSG / CRS are both alphabetically sorted before "Number" so that's why the EPSG selector came up. I improved the detection in a way that it prefers native data types (here number) in favor of specific subtypes like EPSG in case of a conflict.
  3. I also did some refactoring and changing between strings and numbers keeps the values if numerical.

So the behavior should have improved in v0.7.7.

PS: Any data type is allowed to allow conditions to always return a result without throwing an error.

jonathom commented 3 years ago

Any data type is allowed to allow conditions to always return a result without throwing an error.

Thanks! That makes a lot of sense.

select box reset to the first entry

I realize now that with this fixed, the shown type is not as important anymore as values themselves aren't changed anymore. Other improvement sounds great though too! :+1: