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

Remove 'shape' as argument from 'extraterrestrial_r' method in meter_utils.py #64

Closed
raoulcollenteur opened this issue Dec 7, 2023 · 6 comments
Assignees

Comments

@raoulcollenteur
Copy link
Member

Looks like this is a useless argument that could and should be inferred from the data. So I propose to remove it from the argument list.

@raoulcollenteur raoulcollenteur self-assigned this Dec 7, 2023
@raoulcollenteur raoulcollenteur added this to the 1.3: PyEt Reloaded milestone Dec 7, 2023
@mvremec
Copy link
Contributor

mvremec commented Feb 2, 2024

Is implemented with 1.3

@mvremec mvremec closed this as completed Feb 2, 2024
@rodaguayo
Copy link

When I upgraded from v1.2.2 to 1.3.1, my code stopped working (I use dataarrays). There seems to be some sort of latitude issue

@mvremec
Copy link
Contributor

mvremec commented Jun 6, 2024

Hi @rodaguayo,

Thank you for posting this issue! Could you please provide a bit more detail about what you mean by "code stopped working"? Are all of the methods affected, or only specific ones?

I noticed you're working with the Patagonia dataset—glad to see that PyEt has been helpful there!

I will do my best to solve this issue today.

Best,
Matevz

@rodaguayo
Copy link

Hi, thanks for the quick reply!

My workflow is very simple, similar to this tutorial (https://pyet.readthedocs.io/en/latest/examples/09_CMIP6_data.html), with the exception I am using the hargreaves formulation. The error that I recieved is operands could not be broadcast together with shapes because the latitude array only has one dimension. This error was not present in version 1.2.2

Thanks for your time, and for the very helpful package you have developed =)

Best,
Rodrigo

@mvremec
Copy link
Contributor

mvremec commented Jun 8, 2024

HI @rodaguayo,
Thanks again for bringing this to our attention. It seems we removed the code that allowed latitude to be either a 1D or 2D array during our last update. As of version 1.3.1, latitude must be a 2D array, with dimensions matching those of the tmean array (typically latitude/longitude or x/y coordinates).

A quick fix for now is to expand the dimension of the latitude array using:
lat = lat.expand_dims(dim='new_dim', axis=1)

I'll fix this in the upcoming pull request and also add a test to ensure PyEt properly handles both 1D and 2D latitude arrays in future releases. #78

Let me know if this solves your issue.

@rodaguayo
Copy link

Hi @mvremec,

Thanks for this. I imagine something like this could have happened.
Thanks again for your efforts in maintaining ansd developing this helpful package.

Best,
Rodrigo

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

No branches or pull requests

3 participants