Skip to content

Conversation

@soxofaan
Copy link
Member

Issue #391

@soxofaan
Copy link
Member Author

soxofaan commented Oct 31, 2024

todo

  • changelog entry
  • docs
  • more tests?

soxofaan added a commit that referenced this pull request Nov 5, 2024
soxofaan added a commit that referenced this pull request Nov 5, 2024
@soxofaan soxofaan marked this pull request as ready for review November 5, 2024 13:36
@soxofaan soxofaan force-pushed the issue391-multiple-result-nodes branch from b2edbff to 334b819 Compare November 5, 2024 13:38
@soxofaan
Copy link
Member Author

soxofaan commented Nov 5, 2024

I think this is ready to merge, but still a quick review could be handy to make sure there is no mismatch in expectations. @jdries @HansVRP @VincentVerelst

This is quite a large PR in terms of lines of code, but the essential tl;dr is basically described at https://github.com/Open-EO/openeo-python-client/pull/651/files#diff-28f4328e7d44f5afd6d4a6ce10ed344de812d2f2d9460a913c8e65e0a77cdeb5

@jdries
Copy link
Collaborator

jdries commented Nov 6, 2024

approved, one general comment about imports: for our public API, I would prefer to import like:
from openeo import MultiResult
This also works for DataCube for instance, but was apparently also not done for VectorCube. Why do we push the visibility of the 'rest' module?

@soxofaan
Copy link
Member Author

soxofaan commented Nov 6, 2024

from openeo import MultiResult

good point, added

@soxofaan soxofaan merged commit d3407c3 into master Nov 6, 2024
15 checks passed
@soxofaan soxofaan deleted the issue391-multiple-result-nodes branch November 6, 2024 10:10
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.

3 participants