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

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

Closed
wants to merge 1 commit into from

Conversation

noahgolmant
Copy link

@noahgolmant noahgolmant commented Jan 8, 2024

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)

Copy link

google-cla bot commented Jan 8, 2024

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.

@noahgolmant noahgolmant marked this pull request as ready for review January 8, 2024 23:10
@noahgolmant noahgolmant changed the title fix coordinate extension to handle variable return shapes fix coordinate processing to handle variable return shapes from _get_tile_from_ee Jan 8, 2024
@noahgolmant noahgolmant changed the title fix coordinate processing to handle variable return shapes from _get_tile_from_ee fix coordinate processing to handle variable return shapes from image_to_array Jan 8, 2024
@dabhicusp
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

Sounds good, closing this PR!

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.

2 participants