Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #343 +/- ##
==========================================
+ Coverage 95.31% 95.78% +0.46%
==========================================
Files 50 51 +1
Lines 3840 3911 +71
==========================================
+ Hits 3660 3746 +86
+ Misses 180 165 -15 ☔ View full report in Codecov by Sentry. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
|
Thanks for picking this up and turning it into something production-ready! |
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if fbe5543 is merged into master:
|
Collaborator
Author
Thanks for getting it started way back when! It helped a lot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Now that some improved index-handling code is in, I thought I'd take a second attempt at rollups of summaries of variables with indices. This is a PR that will close #43 if it goes forward.
This is based on @jsocolar's #152, except I have iterated on the interface somewhat to be a bit more generic. It allows arbitrary rollup functions to be given on a per-original-summary-column basis, and supplies both overall default rollup functions and summary-specific rollup functions (e.g. having the ess functions rollup with min and rhat functions with max by default).
Demo:
That last bit about
NAs is IMO the weakest part of the API at the moment. Not sure if it needs fixing. One option is to add an overallna.rm = TRUEoption to remove NAs before passing to the rollup functions. Could also allow NAs to be removed from specific summaries with a named argument; likena.rm = c(FALSE, ess_bulk = TRUE)or something like that.Pinging folks who seemed interested in this from previous issues: @avehtari @paul-buerkner @jgabry @jsocolar @andrewgelman
Copyright and Licensing
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses: