epiverse-trace / simulist

An R package for simulating line lists
https://epiverse-trace.github.io/simulist/
Other
5 stars 1 forks source link

Defaults for all arguments in exported functions #120

Closed joshwlambert closed 2 months ago

joshwlambert commented 4 months ago

Would it be a (reasonable) option to add defaults for all arguments here?

Thinking about reducing barriers to usage, being able to run sim_linelist() and getting a line list returned (even if non-informative) may prove helpful to encourage usage. Now it requires going through additional steps that are not trivial - this can increase the barrier to first use enough to just move on if people are pressed for time.

Vice versa, the more accomplished we can make people feel in starting to use the package, the better from my perspective 😄

_Originally posted by @chartgerink in https://github.com/epiverse-trace/simulist/pull/117#discussion_r1616755410_

joshwlambert commented 4 months ago

Comment from @chartgerink from PR #117 relevant to this discussion:

It would be great if getting started only requires

library(simulist)
sim_linelist()

This will encourage the next steps to specify the distributions. Right now this quick start feels not so quick and may be a barrier for people.

joshwlambert commented 3 months ago

I'm happy to implement this change. There is the risk that users will use the functions inappropriately with the default settings and the current setup provides some desirable difficulty, however, as this package is simulating data and not an analytical method used for estimation or inference then inappropriate use should be less problematic.

Discussion on this topic from Epiverse-TRACE (that I could find) are:

which seems to be in favour of providing sensible defaults over having users specify arguments. @Bisaloo do you agree that {simulist} would be worth adding more argument defaults to; and more generally is it Epiverse policy to use defaults wherever possible?

The arguments that would need defaults adding to achieve the simplest function call sim_linelist() outlined by @chartgerink would be:

For sim_linelist() and sim_outbreak():

For sim_contacts():

I propose to use the same anonymous functions as are used in the Get Started vignette.

Also tagging @CarmenTamayo and @adamkucharski to get other opinions from those involve in development discussions.

Bisaloo commented 3 months ago

@Bisaloo do you agree that {simulist} would be worth adding more argument defaults to; and more generally is it Epiverse policy to use defaults wherever possible?

My opinion is not fully formed on this and may continue evolving but my main intuition at this time, which results in this somewhat vague guidance:

Default are fine with the following rules and caveats:

joshwlambert commented 3 months ago

Thanks @Bisaloo. Following your suggestions I think the sim_*() functions in {simulist} are good candidates to have default arguments and I'll make this change.

joshwlambert commented 2 months ago

Closing as default arguments added in PR #149.