Feature ppc ribbon obspoints #257
Conversation
|
@jgabry can you review? Thank you! |
|
This PR may have been a bit premature. What's the documentation policy? Is a standard manual that needs to be updated? What about the vignettes, should some get added? I'm not sure why the tests fail. It seems to be related to installing dependencies. |
|
Hey Charles, really sorry for the delay on this. I just pushed a few commits fixing a few things, but this looks good. I'll add some review comments pointing out the changes. |
R/ppc-intervals.R
Outdated
| #' @param y_draw For ribbon plots only, a string specifying how to draw `y. Can | ||
| #' be `"line"` (the default), `"points"`, or `"both"`. |
There was a problem hiding this comment.
One reason CI was failing was because of a missing doc entry for the y_draw argument. I added that here.
| ppc_ribbon <- function(y, | ||
| yrep, | ||
| x = NULL, | ||
| ..., | ||
| prob = 0.5, | ||
| prob_outer = 0.9, | ||
| alpha = 0.33, | ||
| size = 0.25) { | ||
| size = 0.25, | ||
| y_draw = c("line", "points", "both")) { |
There was a problem hiding this comment.
Not essential, but it's common in R to put all valid options in a vector and then use match.arg later to ensure that what the user wrote is a valid option. (See below where I added match.arg)
There was a problem hiding this comment.
Makes sense. I was wondering what match.arg does.
|
|
||
| style <- match.arg(style) | ||
| y_draw <- match.arg(y_draw) |
There was a problem hiding this comment.
I added this because the match.arg function will result in an error if the user's y_draw argument isn't one of the three valid ones.
Codecov Report
@@ Coverage Diff @@
## master #257 +/- ##
=======================================
Coverage 98.35% 98.36%
=======================================
Files 32 32
Lines 4146 4155 +9
=======================================
+ Hits 4078 4087 +9
Misses 68 68
Continue to review full report at Codecov.
|
|
@jgabry All the changes you made look good to me. Looks like we still have one failing tests, concerning the vignettes. I don't believe I made any changes to those, but if something needs fixing, I can get to it. |
|
The one failing check is unrelated to this PR, so I'm going to go ahead and merge. Sorry for the delay! |
Addresses issue #256
Add an additional argument,
y_draw, forppc_ribbonandppc_ribbon_groupedwhich specifies whether the observed y should be plotted using apoint,line, orboth.While at it, add unit tests for
ppc_ribbon_grouped, emulating what's be done forppc_intervals_grouped.