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 xr_linregress problem with dask #37

Merged
merged 29 commits into from
Dec 17, 2019
Merged

Conversation

jbusecke
Copy link
Owner

I recently encountered a problem when using my trused xr_linregress. This PR establishes tests to isolate the reason.

@codecov
Copy link

codecov bot commented Dec 12, 2019

Codecov Report

Merging #37 into master will increase coverage by 0.04%.
The diff coverage is 41.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
+ Coverage   40.98%   41.02%   +0.04%     
==========================================
  Files          12       12              
  Lines        1320     1321       +1     
  Branches      310      310              
==========================================
+ Hits          541      542       +1     
- Misses        734      737       +3     
+ Partials       45       42       -3
Impacted Files Coverage Δ
xarrayutils/utils.py 31.25% <41.66%> (+0.44%) ⬆️
xarrayutils/_version.py 36.46% <0%> (-0.37%) ⬇️
xarrayutils/plotting.py 17.75% <0%> (ø) ⬆️
xarrayutils/xmitgcm_utils.py 43.54% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b09a2b...11d3d47. Read the comment docs.

@jbusecke
Copy link
Owner Author

It turns out that this problem was related to upstream changes in dask. Until this is fixed upstream, I downgraded test versions to dask < 2.0 and added a warning. This addresses #16.

@jbusecke jbusecke merged commit 5403dd4 into master Dec 17, 2019
@AndrewILWilliams
Copy link

Hi,

I'm keen on using this functionality of xarrayutils, however I'd prefer not to have to downgrade my dask if possible. Is anyone aware of a workaround for this multiple outputs issue? Perhaps via stacking the outputs into a single dimension?

Cheers!

@jbusecke
Copy link
Owner Author

jbusecke commented Mar 4, 2020

If you run xarray 0.15 this works out of the box. If not please let me know

Sent with GitHawk

@jbusecke
Copy link
Owner Author

jbusecke commented Mar 4, 2020

I might want to remove the warning now.

Sent with GitHawk

@jbusecke
Copy link
Owner Author

jbusecke commented Mar 4, 2020

Also thanks for using xarrayutils!

Sent with GitHawk

@AndrewILWilliams
Copy link

If you run xarray 0.15 this works out of the box. If not please let me know

Sent with GitHawk

Beautiful! It works now. Do you know what changed to fix this? Initially there was an issue with ValueError: cannot call vectorize with a signature including new output dimensions on size 0 inputs. Raised in this Github issue.

Thanks again, great package ! :))

@jbusecke
Copy link
Owner Author

jbusecke commented Mar 4, 2020

This was the required fix. As far as I understand when using 'vectorize' dask needed to recieve some metadata of the array shape to prevent it from 'testing' with a 0D input...

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