insightsengineering / chevron

Standard TLGs For Clinical Trials Reporting
https://insightsengineering.github.io/chevron/
Other
11 stars 1 forks source link

Priority order in run method #693

Closed BFalquet closed 10 months ago

BFalquet commented 11 months ago

When arguments are passed to the run method using both ... and user_args, only user_args is taken into account. This causes surprising behavior for the user who has the impression that they can't control the function.

clarkliming commented 11 months ago

honestly I think ... should not be used in the same time with user_args.

some functions use this named argument for easier call (other wise do.call is always needed). an example will be https://github.com/insightsengineering/rtables/blob/8b0192b609de10f26b0983f4876ae787334841cb/R/tt_compatibility.R#L190

and users are always able to modify the arguments through c(user_args, ...)

e.g.

# method 1
run(template, user_args = c(user_args, additional1 = xx, additional2 = yy))

# method 2
run(template, user_args = user_args, additional1 = xx, additional2 = yy)

they do not make a big difference. However I don't see any harm at the moment to also include "..." in user_args.

clarkliming commented 10 months ago

related to run method: is it possible that chevron_tlg inherits function, and make its .Data the run method?

in that case we can even use

aet01(aa,bb,cc) instead of run(aet01, aa, bb,cc) ?

BFalquet commented 10 months ago

We could but it is a very serious refactoring, I am not sure the users would appreciate (in the first version of chevron, it was like that)

clarkliming commented 10 months ago

We could but it is a very serious refactoring, I am not sure the users would appreciate (in the first version of chevron, it was like that)

it is not a very serious refactoring, we can easily make S4 callable; but still this is some wild thoughts and ideas.

prototypes

.chevron_tlg <- setClass(
  "chevron_tlg",
  contains = c("VIRTUAL", "function"),
  slots = c(
    main = "function",
    preprocess = "function",
    postprocess = "function"
  ),
  prototype =  function(...) {run(sys.function(), ...)}
)

still, the run method is called.

run(aet02, ...)
aet02(...)

should work identically