Closed avallecam closed 1 year ago
Thanks, this looks like a nice addition, and I like the worked example. Will try to find someone to review the PR asap
Hello @avallecam! Thank you for submitting a PR. Indeed this looks like a nice addition, though I will point out that it imports a lot of the tidyverse, which will force the packages that depend on incidence to also import these packages. While I like the tidyverse, I don't think incidence is the appropriate package for these features because the tidyverse tends to be relentless in breaking changes.
What I might suggest is to create a separate micro package that provides this interface to the incidence object. This way, incidence can stay (relatively) dependency-light and others can use these features if they want to in their analyses.
What do you think?
Best, Zhian
Hi @zkamvar, I definitely get your point!
Well, as I mentioned in the PR, the functions are hosted in a personal package. However, the best external package would be broom itself. They specify some suggestions to create new tidiers to new model objects here.
Let me know your opinions about this alternative.
Hi there,
@tjtnew is currently working on a revamp of incidence (currently called incidence2) which will use the tidyverse more extensively. We are also planning on re-implementing incidence::fit
etc. in a small separate package using a more consistent interface, more choice of underlying models, and tidier outputs. @avallecam it looks like your contribution would fit nicely in there. If you are fine with it, maybe we can wait until this comes into existence?
Hi there, @tjtnew is currently working on a revamp of incidence (currently called incidence2) which will use the tidyverse more extensively. We are also planning on re-implementing
incidence::fit
etc. in a small separate package using a more consistent interface, more choice of underlying models, and tidier outputs. @avallecam it looks like your contribution would fit nicely in there. If you are fine with it, maybe we can wait until this comes into existence?
totally ok with this too @thibautjombart . so, I will keep tidiers in the current package, for now. @zkamvar @tjtnew let me know if this contribution would still be preferred to be outside the new package. broom requires some details to fulfill but the response is really fast.
Hi @avallecam, we've not yet started on the fitting package but will be in touch when we start to reimplement. FI - the development version of the stripped back "tidy" incidence2 package is hosted at https://github.com/reconhub/incidence2. You might like to have a poke around. The new incidence
class is now a subclass of a tibble so should work well with dplyr verbs. Let us know if you have any feedback.
Cheers
Currently
get_info
generate vector class outputs, isolated from their metadata.For
tidyverse
users, this output could be cleaner, more explicit and easy to manage if the output is a dataframe (or tibble).Here I share some broom-like implementations as
tidy_incidence
andglance_incidence
in a currently personal package here.These functions prove to be easy to manage to any level of stratification, if required, when using purrr with
map
family of functions.Feel free to check them and evaluate their addition to the core package!
Created on 2020-06-12 by the reprex package (v0.3.0)
PD.- this is a RECON workshop alumni, thanks for all you work!