search

pharmaverse / ggsurvfit

http://www.danieldsjoberg.com/ggsurvfit/
Other
67 stars 19 forks source link

Promote tidycmprsk as an Imports dependency #163

Closed dlindelof closed 9 months ago

dlindelof commented 9 months ago

At least two examples rely on tidycmprsk, so some systems cannot build ggsurvfit unless tidycmprsk is a dependency of the package.

ddsjoberg commented 9 months ago

Hi @dlindelof ! Thanks for the PR. If you're building ggsurvfit, you should install all the package dependencies, including those in Suggests. That will ensure all documentation and functions will run as expected. If you run a function that requires tidycmprsk and it's not installed, you'll see a message to install it. Competing risks are called by a low percentage of ggsurvfit users, and we didn't want to bloat their system unneeded pkgs during installation.

dlindelof commented 9 months ago

I see what you mean and I agree; but perhaps you might then want to guard those examples with require(), as is recommended in https://r-pkgs.org/dependencies-in-practice.html#sec-dependencies-in-suggests-in-examples-and-vignettes

ddsjoberg commented 9 months ago

Thanks for the link. Definitely something to consider. I have two thoughts for this: 1. the messaging for users is already quite clear for users when they run the function and the pkg is not installed, 2. i like the clean look without wrapping everything in an if () statement.