Skip to content
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

Work on processes instead of process graphs #155

Closed
m-mohr opened this issue Sep 30, 2020 · 4 comments
Closed

Work on processes instead of process graphs #155

m-mohr opened this issue Sep 30, 2020 · 4 comments

Comments

@m-mohr
Copy link
Member

m-mohr commented Sep 30, 2020

As far as I understand the code, the Python client still seems to mostly work on "process graphs" instead of full processes, which means you'd just pass the "process_graph" part of a full process description to methods and from recent work I've seen it also seems that the JSON output doesn't contain the process metadata, if available. But at least the process graph should be wrapped with {"process_graph": ...}. Not working with the full process description makes portability of processes hard and I'd propose to change this.

@jdries
Copy link
Collaborator

jdries commented Sep 30, 2020

Can you clarify with an example of what you would expect?
We use the client to communicate with validated backends, which seems to work. The fact that I didn't wrap the json was just me doing things manually, but obviously the client will do the right thing when sending a request to a backend.

@m-mohr
Copy link
Member Author

m-mohr commented Sep 30, 2020

Okay, if the JSON serializer outputs {process_graph: ...nodes...} and not just the nodes, than that was a mis-understanding from the recent changes in openeo-usecases.

What confused me was that the methods all have parameters like "process_graph=None" or so, and that sounds like me I'd need to pass just the nodes, not the whole process.

I thought I saw examples just passing nodes, but maybe I just stumbled across old examples? I can't even find it anymore... so maybe nevermind.

Examples for "just nodes" (i.e. the process graph):

{
  "1": {
    "process_id": "example",
    "arguments": {}
  }
}

Example for a full process:

{
  "process_graph": {
    "1": {
      "process_id": "example",
      "arguments": {}
    }
  }
}

@jdries
Copy link
Collaborator

jdries commented Sep 30, 2020

It's indeed the case that internally (between client classes) we could be passing just the nodes, and wrap them in a 'full process' when we send it. But that's more of an internal thing, we anyway try to avoid exposing the graphs themselves to the user.
I may want to add a method that does save a proper json for a given graph, would have helped me avoid confusion!

@soxofaan
Copy link
Member

soxofaan commented Aug 11, 2022

re: in Open-EO/openeo-web-editor#271 @m-mohr suggest to reopen this ticket

However I don't think it is useful to reopen this.
The python client internally works often with "process graphs" instead of "processes", but that is just an implementation detail, a regular user should not be confronted with that.

Legacy-wise there are a couple of DataCube methods that indeed allow the user to export a "process graph" instead of a "process", but these are already flagged as "not recommended to use" in the documentation, e.g.:

The recommended way is using DataCube.to_json which correctly exports a "process" with {"process_graph": ...} wrapper

I'm not sure what else there should be done here. Except maybe making the "not recommended to use" stronger in the docs.
Throwing warnings on usage or something like that is probably not a good idea because the methods are still being used internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants