percentile_doy does not work on non-standard calendars

[tlogan2000]

CFTIME objects do not seem to support 'dayofyear' adn therefore our percetnile_doy does not work for some files ... there is already a fix in xarray (9 days ago)

see :https://github.com/pydata/xarray/pull/2599

@Zeitsperre @huard Is there a way to pull these changes to Ouranos' xarray branch without losing Low's
resampling changes? I have a short-term need to calculate xclim percentile indices (i.e. instead of waiting for low's resampling work to be integrated directly into xarray) If not is there a way to rewrite the percentile_doy function which will work for cftime?

[Zeitsperre]

@tlogan2000 If I understand correctly, you want to merge Low's branch into Ouranosinc/xarray/master as well as merge upstream master? Seeing as we've opened PRs of Ouanosinc/master against upstream master (in both directions) we'll need to open a new branch and try to merge both of them there.

It could get hairy but it's doable. Do you need this ASAP?

[Zeitsperre]

Actually, I just looked things over and the Ouranosinc/master was merged 2 days ago by Low. The fix should be there along with Low's changes which are already in our master. Can you verify this behaviour when building against the Ouranosinc/master xarray?

[tlogan2000]

No I think low's branch is already merged to the Ouranos master. I was thinking of pulling the changes in official Xarray pull request (concerning only the add of 'dayofyear' changes ) without accepting changes to all the other updates that have occurred in official xarray

[sbiner]

Ça ne serait pas plus simple que Low mette ses patchs à xarray par-dessus la nouvelle version de xarray (c. à d. mettre son master à date) en attendant?

On Thu, Dec 20, 2018 at 10:12 AM Trevor James Smith < notifications@github.com> wrote:

@tlogan2000 https://github.com/tlogan2000 If I understand correctly, you want to merge Low's branch into Ouranosinc/xarray/master as well as merge upstream master? Seeing as we've opened PRs of Ouanosinc/master against upstream master (in both directions) we'll need to open a new branch and try to merge both of them there.

It could get hairy but it's doable. Do you need this ASAP?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Ouranosinc/xclim/issues/137#issuecomment-449030370, or mute the thread https://github.com/notifications/unsubscribe-auth/AP4kHN56xG6wciXgfai1phSomoih9izAks5u66jAgaJpZM4ZcSps .

--

Sébastien Biner

[tlogan2000]

If it is super complicated maybe not worth it ... Low is pretty active on submitting directly to xarray.

[Zeitsperre]

I'm just confused because by alla ccounts, the fix should be in our master as they were all merged 2 days ago. Does his branch break other functionalities than the percentile_doyfunctions?

[tlogan2000]

We have Low's updates, but our master is behind the official xarray right? The dayofyear stuff was not added by Low but by the maintainers so I think that our version is missing these adds... In the pull request link it seems as only a few files were changed to add CFTIME dayofyear fucntinality so was thinking it could (maybe) be possible to incorporate those changes wihtout having the major hairiness you mentioned but am not sure...

[Balinus]

I think you can just rebase, if I understand correctly

[Zeitsperre]

Just did a quick and dirty merge of upstream to our master. Check out https://github.com/Ouranosinc/xarray/tree/upstream_dec and let me know if this is what you need.

[huard]

Status ?

[Zeitsperre]

Ouranosinc master is up to date with Low's most recent changes and the xclim dependencies target that specific build. When installing xclim into a fresh environment, ensure that the conda install readout is using the git-installed version.

In terms of whether the percentile_doy function is working, @tlogan2000 @sbiner ?

[tlogan2000]

I think this is taken care of now with the merge of lows resample with xarray master and the doy PR. Can close? @sbiner @huard

[sbiner]

Je pense mais je n'ai pas vécu le problème initial ...