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

Issue #182 add namespace support to DataCube.process and related #183

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

soxofaan
Copy link
Member

@soxofaan soxofaan commented Mar 4, 2021

fixes #182: adds namespace support to DataCube.process, PGNode, ProcessGraphVisitor, GraphFlattener

This improvements changes the ProcessGraphVisitor API changed a bit by adding namespace argument to enterProcess and leaveProcess (which will break openeo-python-driver and openeo-geopyspark-driver)
Before pushing this, I wanted to go through PR+review first

(I bumped version to 0.5.0a1 to clearly mark API change)

bumps version to 0.5.0a1  because the `ProcessGraphVisitor` changed a bit
@soxofaan soxofaan requested a review from jdries March 4, 2021 13:55
@soxofaan
Copy link
Member Author

soxofaan commented Mar 4, 2021

@jdries can you have a quick look if it's ok to merge these PRs?

@jdries
Copy link
Collaborator

jdries commented Mar 4, 2021

Reviewed, the most critical question to me is whether 'namespace' is accepted by openEO partners as a new word in our terminology? We have been using it, but not sure if it was generally presented in the PSC.
@m-mohr Do you happen to know?
Of course, it's not like I immediately have an alternative available.

@soxofaan
Copy link
Member Author

soxofaan commented Mar 4, 2021

"namespace" as a concept and field is already part of the API, e.g. see the <ProcessNode> definition at https://openeo.org/documentation/1.0/developers/api/reference.html#section/Processes/Process-Graphs

it was introduced here: Open-EO/openeo-api#305

@m-mohr
Copy link
Member

m-mohr commented Mar 5, 2021

Yes, that seems all fine. We don't have agreed on Open-EO/openeo-api#348 yet though.

@soxofaan
Copy link
Member Author

soxofaan commented Mar 5, 2021

We don't have agreed on Open-EO/openeo-api#348 yet though.

indeed, but that is not a blocker for this issue #183:
it just introduces function arguments to allow setting the (optional) namespace field in process nodes (following the existing 1.0.0 spec)

@soxofaan soxofaan merged commit 4e58c05 into master Mar 5, 2021
soxofaan added a commit to Open-EO/openeo-python-driver that referenced this pull request Mar 5, 2021
soxofaan added a commit to Open-EO/openeo-geopyspark-driver that referenced this pull request Mar 5, 2021
@soxofaan
Copy link
Member Author

soxofaan commented Mar 5, 2021

thanks!

@soxofaan soxofaan deleted the issue182-namespace-support branch March 5, 2021 11:33
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

Successfully merging this pull request may close these issues.

Add namespace support
3 participants