reviewing calibration vignette #111
Conversation
| IP: This is a bit of a larger point that Steve may want to weigh in on, but the object-oriented approach here makes it a little more difficult to understand how to get help with various functions. For instance, I can't just do `?report` to get info on the arguments for the function above. Perhaps including a short section on the home page of `macpan2` quickly explaining that `macpan2` is object-oriented and how one can get help on object methods would be helpful. I know I had [something about this in the first Quickstart guide](https://canmod.github.io/macpan2/articles/quickstart.html#side-note-macpan2-is-object-oriented), but it didn't go as far as to explain how to get help, which I think is as simple as | ||
|
|
||
| ``` | ||
| > class(sir_simulator) |
There was a problem hiding this comment.
Yes I completely agree @papsti . Something like this in the vignette makes it even more important to make sure that the method documentation is up-to-date. It also makes me want to write a macpan_help function that does basically what you just did.
There was a problem hiding this comment.
can you write your own object-specific help() methods (similar to how one can write custom print methods that get called with print()) as opposed to prefixing with macpan_?
There was a problem hiding this comment.
Yes that's a better idea. But not as S3 methods. It would be more object$help().
There was a problem hiding this comment.
@bbolker @stevencarlislewalker
more comments added inline (prefixed with "IP:") because my great code review experiment didn't work (you can't comment on unchanged lines 😢).
i know you mostly wanted comments about the mk_calibrate() UI but i literally cannot resist commenting on the presentation of the vignette 😄 overall this is a great work in progress, though i am concerned it's getting a little too long. consider splitting into two documents:
- a getting started with calibration vignette that does a simple example (with simulated data) to explain how to work with
mk_calibrate(), followed by the "Hello, World the hard way" section that explains howmk_calibrate()works so that users can customize their own versions - a cookbook/set of case studies for more involved calibrations (all the other sections currently in this vignette
| @@ -61,7 +61,7 @@ mk_sim <- function(init_state = c(S = 99, I = 1, R = 0)) { | |||
| sir_simulator <- mk_sim() | |||
| ## `.phases = "during"` is important so that the number of observations matches the number of time steps | |||
| sir_results = sir_simulator$report(.phases = "during") | |||
There was a problem hiding this comment.
it would be good to point to the documentation that explains what .phases = "during" means here
There was a problem hiding this comment.
as an aside: why is .phases = "during" not the default?
|
@bbolker @stevencarlislewalker just to be clear, i don't think my changes should be pulled in as is since they're mostly comments/suggestions (despite the fact that this is a PR). once these comments are resolved, the changes could be pulled in. |
|
A few responses
|
this PR currently contains no real changes; just setting this up as a PR to enable code review + directly committing changes to the vignette without modifying the article as it appears on the pkg website