[RFC] [R-package] Remove support for passing parameters through '...'

[jameslamb]

Question

I'm opening this request for comment (RFC) to get feedback on the following proposal, originally mentioned in #4202 (comment).

In its next major release, should {lightgbm} remove support for passing parameters through ... in function calls?

This would apply to:

• lgb.cv()
• lgb.Dataset()
• lgb.Dataset.create.valid()
• lgb.train()
• lightgbm()

Description

LightGBM's core library supports many parameters, documented at https://lightgbm.readthedocs.io/en/latest/Parameters.html. The exported functions in {lightgbm} (the R package), support at least 3 different ways to pass values for these parameters.

• in a named list called params
• (for some parameters) via explicit keyword arguments
• via implicit keyword arguments with ...

For example, the following calls to lightgbm::lightgbm() are all equally valid ways to say "perform 14 boosting rounds".

library(lightgbm)

data(agaricus.train, package = "lightgbm")
train <- agaricus.train

# option 1 - params
bst <- lightgbm(
    data = train$data
    , label = train$label
    , objective = "binary"
    , params = list(
        num_iterations = 14L
    )
)

# option 2 - explicit keyword argument ("nrounds")
bst <- lightgbm(
    data = train$data
    , label = train$label
    , objective = "binary"
    , nrounds = 14L
)

# option 3 - passing params through ...
bst <- lightgbm(
    data = train$data
    , label = train$label
    , objective = "binary"
    , num_iterations = 14L
)

Motivation

pros

• reduces friction for new users getting familiar with LightGBM
• turns some situations that are currently warnings into failures ([R-package] clarify parameter documentation (fixes #4193) #4202 (comment))
• reduces maintenance burden of the R package by limiting the number of combinations of inputs that have to be tested

cons

• breaks existing code that relies on the ... behavior
  • NOTE: maintainers have agreed that the next LightGBM release will be a 4.0.0 release, so this breaking change by itself wouldn't force a new major version of LightGBM

References

• {catboost} does not support passing parameters through ...: https://github.com/catboost/catboost/blob/80fa998c355fb9e5e937893e8b7d67286352965c/catboost/R-package/R/catboost.R#L1478
• {xgboost} supports passing parameters through ...: https://github.com/dmlc/xgboost/blob/8760ec48277b345aaaa895b82570c25566fc0503/R-package/R/xgb.train.R#L265
• The process of creating the params list might be made easier in the future by implementing the proposal in [R-package] Add function to generate a list of parameters #4195
• I believe this proposed new behavior would break {treesnip}'s current LightGBM integration: https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L208, although I think that that package could be patched in a way that's compatible with and without this behavior

[jameslamb]

Before we move forward with this change, I'd like to ask some of the people I know to be users of the R package to weigh in. And of course anyone else is welcome to give their feedback on the idea!

• {treesnip} maintainers: @Athospd @dfalbel
• {sl3} maintainers: @nhejazi
• other R community members who've interacted with this project: @jaredlander @david-cortes @mayer79 @mfrasco6 @issactoast @philip-khor @zenggyu @nmnbkhr @TalWac @tonyk7440 @mdancho84 @hcho3

Thanks very much to all of you for your time and effort!

[mayer79]

Thanks for the R support! I think it is a very good idea to simplify the interface in this way. It doesn't only simplify the code but also improves the help in some cases. E.g. for lgb.Dataset I was never sure whether to pass "weight" or "weights".

[TalWac]

Dear developers,
I had only short interaction with the project, however, simplifying the interface will make it easier for new users like me.

Many thanks for the support !

[issactoast]

I am using LGBM via treesnip these days so this affects my code. I would like to wait for opinions from the maintainers of treesnip. However, if the functions support every parameter through params list option, it seems like a good way go.

[jameslamb]

I am using LGBM via treesnip these days so this affects my code.

Thanks! I think that {treesnip} could be patched in a way that wouldn't break existing code using that package. Basically today {treesnip} collects any keyword arguments you pass through ... of treesnip::train_lightgbm() and passes them to lightgbm::lgb.train().

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L208

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L267

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L278

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L283

I think it could be changed to instead collect those and pass them to params (making special exceptions for the existing explicit keyword arguments like nrounds). If we move forward with the proposal in this RFC, I'm willing to go do the work to create such a patch for {treesnip}, to avoid breaking code that uses that project.

[david-cortes]

I am using LGBM via treesnip these days so this affects my code.

Thanks! I think that {treesnip} could be patched in a way that wouldn't break existing code using that package. Basically today {treesnip} collects any keyword arguments you pass through ... of treesnip::train_lightgbm() and passes them to lightgbm::lgb.train().

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L208

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L267

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L278

https://github.com/curso-r/treesnip/blob/bf27cd871b7fb663a76818f66ea0a5f9bf09f444/R/lightgbm.R#L283

I think it could be changed to instead collect those and pass them to params (making special exceptions for the existing explicit keyword arguments like nrounds). If we move forward with the proposal in this RFC, I'm willing to go do the work to create such a patch for {treesnip}, to avoid breaking code that uses that project.

That wouldn't break the existing code, since arguments which are in ... would be assigned to the named slots in the function call. I think LightGBM wouldn't accept unnamed arguments so if there wasn't any issue before, there shouldn't be an issue now.

[jameslamb]

@david-cortes I don't think that's correct. Consider my example at the top of this issue with num_iteration. There is no named keyword argument num_iteration in the signature of lightgbm::lgb.train()...but passing it that way works today because lgb.train() assumes any keyword arguments not matched to one of its formal arguments must be a valid parameter from https://lightgbm.readthedocs.io/en/latest/Parameters.html.

LightGBM/R-package/R/lgb.train.R

Lines 86 to 87 in 5014f19

	additional_params <- list(...)
	params <- append(params, additional_params)

To be clear, this proposal is not about adding one keyword argument to lgb.train() per parameter in https://lightgbm.readthedocs.io/en/latest/Parameters.html.

[jaredlander]

I think one of the biggest complaints about {xgboost} is the interface. The choice of a params list and the ... is clunky and leaves you with no autocomplete in an IDE.

A params list feels very outdated for R. I think it's still popular in Python, but not so much in R anymore. If you do go this route, rather than making it a list where the user can enter anything and has no guidance, make it a function whose arguments are the parameters. An example can be seen in tune::control_grid(), which isn't for tuning parameters, but same idea. Since it's a function the user can use autocomplete and get in-context help. And I now see this is what you were talking about in #4195.

Though generally, I like keyword arguments in the function itself. This means I can use autocomplete in the IDE and all the documentation is in one place and feels less clunky than a params list. That said, I know it is possible to get autocomplete to work even with ... and same for documentation. But I'm not sure what magic RStudio uses for that.

As an aside, you used nrounds as a keyword argument but num_iterations for the other examples. Is the parameter name context specific?

[jameslamb]

Thanks very much!

A params list feels very outdated for R.... #4195.

Yes, #4195 is the proposal for exactly what you're talking about.

To be clear, removing the interface of passing a params list is not up for debate in this RFC. This is about the narrow question of removing supporting for one of 3 different ways you can currently pass parameters (through ...).

If there is some other API change you'd like to see in this project's R package (besides #4195), we'd welcome issues describing a proposal and how it is an improvement over the current state.

That said, I know it is possible to get autocomplete to work even with ... and same for documentation. But I'm not sure what magic RStudio uses for that.

I think you're probably talking about how some packages use the {roxygen2} tag #' @inheritDotParams. (examples can be seen in {roxygen2}'s tests: https://github.com/r-lib/roxygen2/blob/6c1e42f061e9435a14bd3a0e01c7f08fc7aa9160/tests/testthat/test-rd-inherit.R#L473).

With that tag, if you set up your stuff like this:

#' @name some_func
#' @inheritDotParams data.table::data.table
some_func <- function(...) {
    someDT <- data.table::data.table(...)
    return(someDT)
}

{roxygen2} will fill out all of the documentation for individual parameters from data.table::data.table(). I don't think that's directly applicable here since {lightgbm} isn't using ... to say "other arguments all passed to exactly one other R function".

As an aside, you used nrounds as a keyword argument but num_iterations for the other examples. Is the parameter name context specific?

In the earliest days of this project's R package, some explicit keyword arguments were added for functions like lgb.train() which can also be specified in params. Unfortunately sometimes those names differed. In this case, the keyword argument nrounds means exactly the same thing as the parameter num_iterations recognized by LightGBM core (https://lightgbm.readthedocs.io/en/latest/Parameters.html#num_iterations). I understand that that is a source of confusion and maybe resolving that difference could also be considered as part of the 4.0.0 release, but that is not related to this RFC so I'd prefer to have that discussion in a separate issue.

[mayer79]

We might add a depreciation note in the next R release, like so

if (length(list(...)) {
  warning("Starting from the next major release of LightGBM, passing arguments through ... will be deprecated.")
}