-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Refactor ECDF code #2311
Refactor ECDF code #2311
Conversation
This reverts commit 780d395.
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.
This is amazing, thanks!
After this it will probably be possible to use these functions in plot_ppc(kind="cumulative") and in plot_loo_pit instead of each having their own subset of ecdf functionality
|
||
|
||
@pytest.mark.parametrize( | ||
"dist", [scipy.stats.norm(3, 10), scipy.stats.binom(10, 0.5)], ids=["continuous", "discrete"] |
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 didn't know the ids
argument existed, I love it!
Yeah! I also think it would be great if the plots where one could use I've made all requested changes. |
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.
left one last documentation related comment but looks great
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2311 +/- ##
==========================================
+ Coverage 86.73% 86.79% +0.05%
==========================================
Files 122 123 +1
Lines 12705 12730 +25
==========================================
+ Hits 11020 11049 +29
+ Misses 1685 1681 -4 ☔ View full report in Codecov by Sentry. |
Description
This PR is a major refactor of the ECDF plot code to remove dead code, reduce code duplication, use more descriptive variable/function names, and make it easier to make the changes detailed in #2309. It makes no changes to the functionality of
plot_ecdf
.All utility functions have been moved to
arviz.stats.ecdf_utils
. It might be worth exportingecdf_confidence_band
, but this can be left for a future PR.Example
This example shows that public functionality is unchanged.
On main
This PR
Checklist
📚 Documentation preview 📚: https://arviz--2311.org.readthedocs.build/en/2311/