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

include xr-ed versions of properscorings crps_gaussian, crps_ensemble #10

Merged
merged 8 commits into from
May 14, 2019
Merged

include xr-ed versions of properscorings crps_gaussian, crps_ensemble #10

merged 8 commits into from
May 14, 2019

Conversation

aaronspring
Copy link
Collaborator

@aaronspring aaronspring commented May 4, 2019

  • probabilistic.py: xr.apply_ufunc on properscoring.crps_gaussian and properscoring.crps_ensemble and properscoring.threshold_brier_score
  • test_probabilistic.py:
    • test whether same numerical result as inside properscoring
    • test whether xr_crps_x returns chunked arrays when input dask
    • no testing of properscoring itself (aka. no np_probabilistic)

@aaronspring aaronspring closed this May 4, 2019
@aaronspring aaronspring reopened this May 4, 2019
@aaronspring aaronspring marked this pull request as ready for review May 4, 2019 10:11
@aaronspring aaronspring mentioned this pull request May 4, 2019
9 tasks
@raybellwaves
Copy link
Member

LGTM.

Did you mean to leave the doc string for xr_crps_gaussian and xr_crps_ensemble empty?

Mind padding them out like https://github.com/raybellwaves/xskillscore/blob/master/xskillscore/core/deterministic.py#L10L33 ?

@aaronspring
Copy link
Collaborator Author

Stephan Hoyer suggested to have this wrapper in xskillscore.

@aaronspring
Copy link
Collaborator Author

also added threshold_brier_score. unfortunately I didnt get apply_ufunc for the different cases of threshold (scalar works, but for an 1d array threshold I get chunking errors.) However, the user can bypass this by looping over different threshold levels if thats needed. Ready to merge from my side.

@raybellwaves
Copy link
Member

Thanks a lot. The only thing I can see is the long line here re. PEP8
https://github.com/aaronspring/xskillscore/blob/AS_probabilistic_properscoring/xskillscore/core/probabilistic.py#L70

Run flake8 on the codebase as there may be other lint in there

FYI I learnt this here: dask/dask-jobqueue#78 (comment)

@aaronspring
Copy link
Collaborator Author

sorry. forget to beautify.

@raybellwaves raybellwaves merged commit 83f61cc into xarray-contrib:master May 14, 2019
@raybellwaves
Copy link
Member

Cheers @aaronspring!

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