search

google / Xee

An Xarray extension for Google Earth Engine
Apache License 2.0
240 stars 28 forks source link

fix coordinate processing to handle variable return shapes from `image_to_array` #123

Closed noahgolmant closed 8 months ago

noahgolmant commented 8 months ago

In some cases, lat and lon would have different return shapes from get_tile_from_ee. This resulted in an error during np.concatenate like:

ValueError: all the input arrays must have same number of dimensions, but the array at index 0 has 1 dimension(s) and the array at index 1 has 0 dimension(s)

This fixes the concatenation logic to just store a flattened array of coordinates. I think this should also resolve the same issue as #121, but I think it's slightly more robust since it supports all possible return shapes.

(I just went ahead and signed the CLA after putting this up for draft btw)

google-cla[bot] commented 8 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

dabhicusp commented 8 months ago

It seems like this approach might not always work as expected. Threads operate independently and might produce output at different times. For our requirement, maintaining the order of output is crucial, but the solution you wrote above is not capturing the index or order of the output.. Therefore, I'm concerned that this method might not deliver the expected output every time.

Also do you mind if I make use arr.flatten() in my #121, this seems better than what we are currently using tiles[i] = arr.tolist() if coordinate_type == 'x' else arr.tolist()[0] ? Thanks.

noahgolmant commented 8 months ago

I see what you mean. Could we just sort the values before returning? Since lat/lon must be ordered. Or just modify the existing code to use tiles[i] without the other per band logic you introduced?

dabhicusp commented 8 months ago

Not really sure if we should sort the data as sorting will take time so it will increasing over-all opening time and regarding the second-part in the existing code i of tiles[i] is slice object and it gives error in the example of this #118 and #121 is updating that i to normal index and resolve that error.

I'm taking reference of your PR & incorporating tiles[i] = arr.flatten() in #121. Hope you don't mind. Thanks.

noahgolmant commented 8 months ago

Sounds good, closing this PR!