Open-EO / openeo-python-client

Python client API for OpenEO
https://open-eo.github.io/openeo-python-client/
Apache License 2.0
155 stars 41 forks source link

Context parameter in openeo.UDF.from_file() causes confusion #520

Open JeroenVerstraelen opened 10 months ago

JeroenVerstraelen commented 10 months ago

A user let us know that the following code does not pass the context to his UDF:

cube = cube.apply_neighborhood(
    openeo.UDF.from_file("test_udf.py"),
    size=[
        {"dimension": "x", "unit": "px", "value": 32},
        {"dimension": "y", "unit": "px", "value": 32}
    ],
    overlap=[],
    context={"test_variable": "Definitely not None"}
)

The solution was to pass the context in the from_file method instead:

cube = cube.apply_neighborhood(
    openeo.UDF.from_file("test_udf.py", context={"test_variable": "Definitely not None"}),
    size=[
        {"dimension": "x", "unit": "px", "value": 32},
        {"dimension": "y", "unit": "px", "value": 32}
    ],
    overlap=[],
)

We should either:

soxofaan commented 9 months ago

I'd like to point out that this is not only a client issue, but a less obvious part of openeo process graphs with run_udf.

There are multiple ways to get "configuration" or context into a UDF:

Directly in run_udf:

 {
   "process_id": "run_udf"
    "arguments": {
      ...
      "context": {"color": "red"}     

or through the parent process, e.g. apply

  {
    "process_id": "apply",
    "arguments": {
      ...
      "process": {
        ...
        "runudf1": {
          "process_id": "run_udf",
          "arguments": {
            "context": {"from_parameter": "context"}
            ...
        .....
      "context": {"color": "red"}  

note that the context from apply has to be passed through explicitly by run_udf with {"from_parameter": "context"}. If you forget that passing through you get in the situation mentioned above

the following code does not pass the context to his UDF ...

The first solution (direct context in run_udf) is obviously the easiest, e.g. as proposed above too

solution was to pass the context in the from_file method instead

However, the solution with passing through the context from "upstream" is still important to cover, e.g. in UDPs or services where the context or parts of the context come from (UDP) parameters.

soxofaan commented 9 months ago

regardless, we can at least throw warnings client side when it looks like the user might have forgotten something to get the context inside the UDF properly

soxofaan commented 9 months ago

an alternative solution is to use {"from_parameter": "context"} by default in run_udf if there is a context set by the parent process.

soxofaan commented 9 months ago

This is also something we could check for (e.g. in /validation or runtime warning) back-end side

jdries commented 9 months ago

Note that most of our examples in python client do not show how to use the context. Just adding that there would probably help a lot, as users can then just copy-paste a working example?

soxofaan commented 1 month ago

an alternative solution is to use {"from_parameter": "context"} by default in run_udf if there is a context set by the parent process.

=>