Fix gee example #128

bgoesswe · 2020-04-01T14:31:13Z

Fixed GEE example for the 1.0.0rc2 version of it.
Made some changes in the client to be compliant with 1.0.0rc2.
Added the possibility to apply complex apply process graphs.
Implemented multiple file result download.

…e Python client

soxofaan · 2020-04-01T15:14:16Z

openeo/imagecollection.py

@@ -52,7 +52,14 @@ def bands(self) -> List[Band]:

    def _get_bands(self) -> List[Band]:
        # TODO refactor this specification specific processing away in a subclass?
-        # First try `properties/eo:bands`
+        # First try `summaries/eo:bands`
+        eo_bands = self.get('summaries', 'eo:bands')


ah ok, I didn't know about this summaries field as replacement for the properties field.
Maybe we can drop support for properties in favor of summaries. I have no idea if it is necessary to keep supporting that

Yes, also "cube:dimensions" is now one layer up in the dictionary, I updated that in my last commit.

soxofaan · 2020-04-01T15:20:29Z

openeo/rest/connection.py

+        if self._api_version.at_least("1.0.0"):
+            request = {"process": {"process_graph": graph}}
+        else:
+            request = {"process_graph": graph}


reference for self: Open-EO/openeo-python-driver#33

soxofaan · 2020-04-01T15:20:50Z

openeo/rest/connection.py

            "title": title,
            "description": description,
            "plan": plan,
            "budget": budget
        }
+
+        if self._api_version.at_least("1.0.0"):
+            pg_meta["process"] = {"process_graph": process_graph}


reference for self: Open-EO/openeo-python-driver#33

soxofaan · 2020-04-01T15:22:55Z

openeo/rest/datacube.py

+            if isinstance(process, str):
+                arguments["process_graph"] = PGNode(
+                        process_id=process,
+                        arguments={data_argument: {"from_parameter": "x"}})


for 0.4 it has to be from_argument I think

Yes true, done in latest commit

soxofaan · 2020-04-01T15:26:04Z

openeo/rest/datacube.py

        ))

-    def reduce_temporal_simple(self, process_id="max") -> 'DataCube':
+    def reduce_temporal_simple(self, process_id="max", dim_abbr="temporal") -> 'DataCube':


I would use dimension instead of dim_abbr, as used in other places in DataCube methods

also, picking the correct dimension should be done automatically instead of manually by user: see #116. Can you put a comment # TODO #116 determine dimension based on datacube metadata here as well?

soxofaan · 2020-04-01T15:30:56Z

openeo/rest/datacube.py

@@ -711,6 +725,10 @@ def merge(self, other: 'DataCube', overlap_resolver: PGNode = None) -> 'DataCube
            'cube2': {'from_node': other._pg},
        }
        if overlap_resolver:
+            # TODO: for 1.0.0 support


Can you add #125 in this TODO line? makes it easier to find 1.0 related TODOs

or wait: as far as I understand the spec, there is no need for additional process level here. I think this is correct:

... "overlap_resolver": { "process_graph": { "max": { "process_id": "max" ....

Yes you are right, I removed the comments

soxofaan · 2020-04-01T15:35:57Z

openeo/rest/datacube.py

+                    process_id=func,
+                    arguments={"data": {"from_parameter": "data"}}
+                )}
+        # TODO: Might be necessary for 1.0.0rc2


same as above: from my understanding of spec this additional "process" level is not necessary

soxofaan · 2020-04-01T15:46:35Z

openeo/rest/job.py

-        """ Download job results."""
+    def download_results(self, target: Union[str, pathlib.Path] = None) -> list:
+        """
+        Download job results to the target folder path.


This feels like a confusing API (especially the interplay between target and how many files there are in the result)

maybe we should provide instead:

download_result to download a single result: fails when target is a folder or there are multiple results

download_results is more generic: target should always be a folder

Now there are these two functions in the job class.

soxofaan · 2020-04-01T15:48:41Z

openeo/rest/job.py

-        return target
+
+        file_list = []
+


put a line target = Path(target or Path.cwd()) here, then you don't have to do if target anymore in the code below

soxofaan · 2020-04-01T15:51:14Z

openeo/rest/job.py

-                handle.write(block)
-        return target
+
+        file_list = []


isn't it more logical to use a "to download" dict (mapping hrefs to local paths) instead of a list file_list?

soxofaan · 2020-04-02T09:06:00Z

to address all "process"/"process_graph" changes I created dedicated ticket #129

soxofaan · 2020-04-02T10:41:31Z

Inspired by this PR, I worked out the "process"/"process_graph" issue a bit more in PR #131
Unfortunately this will conflict with this (PR #128 )

If you want @bgoesswe , I can merge #131 and rebase #128 to get a view on the remaining issues

bgoesswe · 2020-04-02T10:57:24Z

Yes sure that is a good idea.

…-EO#128)

- based on Open-EO#128 - more flexible file/folder handling - automatically create parent folder - support multiple result files: `download_results` vs `download_result` - add unit tests

soxofaan · 2020-04-03T13:27:23Z

FYI: I just merged a rework of this PR in master

soxofaan · 2020-04-03T14:09:12Z

Hi @bgoesswe I think I merged now all your changes (with some finetuning here and there) into master (including commit added TUW-UseCase to examples).
Can you check if the your use cases work still on master as expected?

Because of the finetuning, this PR #128 is conflicting, and I think it is best to close it.

If you have more additions it is probably best to start with a new branch starting from master

soxofaan · 2020-04-03T14:11:27Z

another thing (one of the finetunings I did), is simplification of this pattern:

job = datacube.send_job()
print(job.job_id)
print(job.start_job())
print(job.describe_job())
time.sleep(30)
print(job.download_results("/tmp/"))

you can now just do

job = datacube.send_job()
res = job.start_and_wait().download_results("/tmp")
print(res)

instead of a hardcoded sleep, it will do a poll loop until job is finished

bgoesswe · 2020-04-03T14:28:43Z

Yes works fine, thanks !

…pectral_bands", "temporal") - bump version so users can stick to older client version if dimension name checks are too aggressive for their backend - recommended temporal dimension name is "t" - recommended bands dimension name is "bands" other refs: Open-EO#123, Open-EO#132, Open-EO#128

…, "temporal") - bump version so users can stick to older client version if dimension name checks are too aggressive for their backend - recommended temporal dimension name is "t" - recommended bands dimension name is "bands" other refs: #123, #132, #128

bgoesswe added 4 commits March 30, 2020 15:17

upgraded GEE example to 1.0.0rc2, therefore made some adaptions to th…

26190cb

…e Python client

fixed GEE python examples for version 1.0.0rc2, improved apply process

6372892

fixed bug in apply with process string

3da4b9c

removed testing lines

9895e7e

bgoesswe requested a review from soxofaan April 1, 2020 14:31

soxofaan reviewed Apr 1, 2020

View reviewed changes

soxofaan mentioned this pull request Apr 2, 2020

Update process graph (POST) requests to 1.0 API #129

Closed

8 tasks

fixed pull request issues, updated tests and datacube for 1.0.0rc2

3c67ed0

soxofaan mentioned this pull request Apr 2, 2020

improve 1.0-style process graph submit requests #131

Merged

soxofaan mentioned this pull request Apr 2, 2020

Fix/update collection band metadata handling (1.0) #132

Closed

soxofaan pushed a commit to soxofaan/openeo-python-client that referenced this pull request Apr 3, 2020

Cherry picking from Open-EO#128

5bd125f

soxofaan added a commit to soxofaan/openeo-python-client that referenced this pull request Apr 3, 2020

DataCube.apply: allow non-simple process graph (cherry pick from Open…

c3f5bf5

…-EO#128)

added TUW-UseCase to examples

151ea8f

soxofaan pushed a commit to soxofaan/openeo-python-client that referenced this pull request Apr 3, 2020

Cherry-picking Open-EO#128: upgraded GEE example to 1.0.0rc2

6bc3ff1

soxofaan added a commit to soxofaan/openeo-python-client that referenced this pull request Apr 3, 2020

More finetuning of RESTJob.run_synchronous (Open-EO#128, Open-EO#135)

b874b30

soxofaan added a commit that referenced this pull request Apr 3, 2020

Merge rework of #128 and related tweaks

e0aa3b5

soxofaan closed this Apr 3, 2020

soxofaan mentioned this pull request Apr 3, 2020

job outputs: support single file vs multiple file results better #135

Closed

soxofaan deleted the fix_gee_example branch April 15, 2020 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix gee example #128

Fix gee example #128

bgoesswe commented Apr 1, 2020

soxofaan Apr 1, 2020

bgoesswe Apr 2, 2020

soxofaan Apr 1, 2020

soxofaan Apr 1, 2020

soxofaan Apr 1, 2020

bgoesswe Apr 2, 2020

soxofaan Apr 1, 2020

soxofaan Apr 1, 2020

soxofaan Apr 1, 2020

bgoesswe Apr 2, 2020

soxofaan Apr 1, 2020

soxofaan Apr 1, 2020

bgoesswe Apr 2, 2020

soxofaan Apr 1, 2020

soxofaan Apr 1, 2020

soxofaan commented Apr 2, 2020

soxofaan commented Apr 2, 2020

bgoesswe commented Apr 2, 2020

soxofaan commented Apr 3, 2020

soxofaan commented Apr 3, 2020

soxofaan commented Apr 3, 2020

bgoesswe commented Apr 3, 2020

Fix gee example #128

Fix gee example #128

Conversation

bgoesswe commented Apr 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soxofaan commented Apr 2, 2020

soxofaan commented Apr 2, 2020

bgoesswe commented Apr 2, 2020

soxofaan commented Apr 3, 2020

soxofaan commented Apr 3, 2020

soxofaan commented Apr 3, 2020

bgoesswe commented Apr 3, 2020