-
Notifications
You must be signed in to change notification settings - Fork 48
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
make fit for xr=0.15.1 #348
Conversation
climpred/reference.py
Outdated
@@ -89,7 +89,8 @@ def compute_persistence( | |||
# at lead 0 is == 1. | |||
if [0] in hind.lead.values: | |||
hind = hind.copy() | |||
hind['lead'] += 1 | |||
with xr.set_options(keep_attrs=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First time I use this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never get it to work consistently, but I'm glad it does here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a small comment as to what this fixes? What attributes were being dropped?
climpred/tests/conftest.py
Outdated
da['lead'] = da['lead'] + 1 | ||
da['init'] = da['init'] - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the plus and minus are backwards here and should be fixed. Lead goes back 1 to start at zero; init goes forward one since lead was modified to start at zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for finding this. however the tests didnt find this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two very small changes. A comment before the with xr.options()
line and a potential typo for conftest
.
@aaronspring , also make sure to require |
We don’t require 0.15.1 with these changes. Do we? |
We do so that the |
forgot about skipna in quantile. True your right |
Make sure to add to |
Looks good, thanks @aaronspring ! |
Description
xr.quantile
introduce rechunk_to_single_chunk_if_more_than_one_chunk_along_dim
, because we had this inmy_quantile
, see I tried to re-implement:https://github.com/bradyrx/climpred/blob/8be0a8939b15d248939b4aa45244a0d9617af290/climpred/bootstrap.py#L58-L62 This maybe have triggered some calculations. I now reimplemented as is and therefore got rid of
my_quantile
and put this piece before `xr.quantileCloses #316
Type of change
Please delete options that are not relevant.
Pre-Merge Checklist (final steps)