Conversation
…for the terminology section up to and including opposing rule breaks but not overhanging reversions. Also added bookdown for cross-referencing.
…strate overhanging reversion in the SSA vignette
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
derrynlovett
left a comment
derrynlovett
left a comment
There was a problem hiding this comment.
Overall the vignette reads well, and functions as expected.
Suggested changes are for grammar, an additional reference if available, and minor changes to plot outputs.
If appropriate, and comments are clear enough, I am happy to make edits and push the changes.
| on this dataset see `?ed_attendances_monthly`. | ||
|
|
||
| ```{r 2.0, fig.width=7, fig.height=9} | ||
| ```{r extending-limits, fig.width=7, fig.height=9, fig.cap="Extending baseline control limits"} |
There was a problem hiding this comment.
Consider reducing fig.height to make faceted plots more readable.
fig.height=7
| @@ -132,7 +242,7 @@ they? In fact, various authors offer guidance on what such a minimum should be, | |||
| with values ranging from 17 to 25 points. In `plot_auto_SPC()`, $n_{min}$ is | |||
| specified by the `periodMin` argument, defaulting to 21. | |||
There was a problem hiding this comment.
Is there a reference for why periodMin = 21 is chosen for autospc?
There was a problem hiding this comment.
I can add this, good idea
There was a problem hiding this comment.
Couldn't add comment to unchanged code.
Line 281, suggest add "is" to read:
i. If there is at least...
There was a problem hiding this comment.
Is it possible, and valid, to change the y-axis lower limit to remove whitespace, rather than starting at 0?
Particularly in Figure 2.1 where 0 to ~8,000 is just whitespace in each of the facet plots.
There was a problem hiding this comment.
At present the override_y_lim argument of plot_auto_SPC() for some reason only controls the upper limit of the y-axis. This should be changed though - are you ok to add an issue for this? I suggest leaving as is until this new feature is implemented. May not have time before publication of the paper but let's see.
|
Thanks for this @derrynlovett very useful comments. Happy for you to make any changes you're happy to do and push here - once you're done I'll add that explanation about |
Amended figure height and grammatical changes.
|
@derrynlovett I've added that explanation re. the |
Captures additional packages required to use the bookdown::html_document2 output type for vignettes.
2 participants
Improvements to Stable Shift Algorithm vignette
Closes #176