pharmaR / riskmetric

Metrics to evaluate the risk of R packages
https://pharmar.github.io/riskmetric/
Other
161 stars 31 forks source link

Consider adding a wrapper function #276

Open parmsam-pfizer opened 1 year ago

parmsam-pfizer commented 1 year ago

The wrapper function would cover the three main steps of the workflow below (maybe called pkg_risk()). I imagine that most users are just using this workflow, so it makes sense to me to provide a wrapper function. The underlying functions would still be available for users who need to drill down further. Creating issue to continue discussion from today's sprint meeting.

pkg_ref(c("riskmetric", "utils", "tools")) %>%
  pkg_assess() %>%
  pkg_score()
emilliman5 commented 1 year ago

What are the potential benefits of having a single wrapper function, beside easy of use? Our current paradigm follows the normal declarative flow using pipes, which many (if not most) users would consider canonical R style.

parmsam-pfizer commented 1 year ago

I would agree with that the workflow is straightforward, but I wonder how many users actually need to inspect the objects from pkg_ref() and pkg_assess(), if they're only interested in the pkg_score() output. Having the function available would make it easier for first time users to generate the risk scores with less lines of code. Also, reoccurring users wouldn't need to remember the function sequence to get the scores tibble. There is a risk, as you said, that folks might be inclined to use the package without understanding it, but I think that we still expect the target audience to read the documentation.

Do we have insight on how other packages have approached such a problem? I know that logrx has taken a similar approach with their axecute() wrapper function.

ddsjoberg commented 1 year ago

FWIW, as an outsider, it took some time to figure out how to get my risk score, and a wrapper would be great!

Maybe an S3 method, pkg_score.character() could be dispatched so users could simply use pkg_score('riskmetric'). The method would call pkg_ref(x) |> pkg_assess() |> pkg_score()

dgkf commented 1 year ago

Love these ideas, and I think I'm generally on-board.

In interface design, I often follow the perl ethos of "make the simple things work and the hard things possible", and this feels like a perfect application of that design style.

I think there are certainly benefits to having more comprehensive interfaces (via pkg_ref, pkg_assess), but in many cases - especially for new users - just getting the thing to work is the first main goal.

My only reservations would be regarding how we can highlight the assumptions that get baked into the default interpretation of a character package name. If we think there's some risk of assumptively assessing a local package vs a CRAN remote or a package from source, then it just becomes more important that these qualities are more visible when doing the "simple thing" as to nudge people toward tweaking the behavior to answer the harder, or more explicit, questions.

ddsjoberg commented 1 year ago

I don't know if this helps, but even without the wrapper function I had no idea if it was checking the CRAN or local version of the package 😆 🤣 😆 🤣

Maybe some {cli} messaging (in both cases) could be useful?

parmsam-pfizer commented 1 year ago

Thanks for the feedback! I'll bring this up in our next dev team meeting.