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

Stats func accessor 59 #99

Merged
merged 17 commits into from
Nov 25, 2020
Merged

Stats func accessor 59 #99

merged 17 commits into from
Nov 25, 2020

Conversation

lukegre
Copy link
Contributor

@lukegre lukegre commented Nov 5, 2020

Description

Added xarray accessors for the stats functions. Added accessor as stats to xr.DataArray (and not xr.Dataset).
Includes: corr, linear_slope, linregress, polyfit, rm_poly, rm_trend

Did not change the API of these functions as I originally intended - I didn't realise how extensive the tests were and this would all have to be changed 😵

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Manually. Not a complicated implementation of xarray accessors.

Checklist (while developing)

  • I have added docstrings to all new functions.
  • I have updated the sphinx documentation, if necessary.

@lukegre
Copy link
Contributor Author

lukegre commented Nov 5, 2020

Travis tests are failing on stats.rm_poly for the dask arrays when called with x and y. When y is forced to load(), the tests pass. Not sure why the docs build is failing.
I have not made any changes to stats functions.

@aaronspring
Copy link
Collaborator

please raise a PR on the failing stats functions. new versions of xr and/or dask likely the reason

@bradyrx
Copy link
Owner

bradyrx commented Nov 13, 2020

please raise a PR on the failing stats functions. new versions of xr and/or dask likely the reason

@aaronspring I feel like we recently dealt with this in climpred? There was some break due to the new versions and I can't remember what we had to change. I think you were working on it.

@bradyrx bradyrx mentioned this pull request Nov 13, 2020
* Add Github Actions system for CI
* Removes Travis CI
* Switches to CodeCov for coverage testing
* Fixes bug with vectorize=True with new dask/xarray
@bradyrx
Copy link
Owner

bradyrx commented Nov 13, 2020

@luke-gregor, if you rebase this onto master the bug should be fixed and you should be good to go to proceed with this.

@bradyrx
Copy link
Owner

bradyrx commented Nov 17, 2020

This will be fixed from #105. You can rebase once I merge it when the RTD tests pass and so on. Sorry this little implementation has been such a headache! But you found a few holes with recent upstream changes. :)

@bradyrx
Copy link
Owner

bradyrx commented Nov 17, 2020

Okay @luke-gregor, another rebase onto master should do the trick here!

@lukegre
Copy link
Contributor Author

lukegre commented Nov 18, 2020

All passed! I'll update the docs accordingly.

@bradyrx
Copy link
Owner

bradyrx commented Nov 25, 2020

Thanks @luke-gregor! Sorry for the delay here. In the final stretch of writing, so haven't kept too close of an eye on Github. Merging now!

@bradyrx bradyrx merged commit 20c005d into bradyrx:master Nov 25, 2020
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.

3 participants