[R] Stop when handling deprecated parameters.

[trivialfis]

Ran into the following error when using the old watchlist parameter:

Error in parse(text = paste(new_par, "<-", pars[[pars_par]])) : 
  <text>:1:10: unexpected '<'
1: evals <- <
             ^
Calls: xgb.train -> check.deprecation -> eval -> parse
In addition: Warning message:
In check.deprecation(...) : 'watchlist' is deprecated.
Use 'evals' instead.
See help("Deprecated") and help("xgboost-deprecated").
Execution halted

This is caused by the R <pointer ..> notation used for the DMatrix object. The PR drops the compatibility with old parameter names as it's quite difficult to make robust. A clear stop might be preferred.

After the PR:

Calls: xgb.train -> check.deprecation
In addition: Warning message:
In check.deprecation(...) : 'watchlist' is deprecated.
Use 'evals' instead.
See help("Deprecated") and help("xgboost-deprecated").
Execution halted

[trivialfis]

cc @mayer79 @david-cortes

I'm looking into updating the integration with mlr3 and see how difficult it is. Upstreaming this small patch, still learning.

[david-cortes]

Sorry that I'm a bit late to the discussion.

How about putting something in the error message?

Also, if the issue is about spaces in names of variables, perhaps it could be solved by using substitute or similar tricks instead of an eval of a text command.

Do you have some reproducer for the error as it was before merging this PR?

[trivialfis]

Will share an example once I get back.

[trivialfis]

@david-cortes

library(xgboost)

data(agaricus.train, package = "xgboost")
data(agaricus.test, package = "xgboost")

nthread <- parallel::detectCores()

dtrain <- with(
  agaricus.train, xgb.DMatrix(data, label = label, nthread = nthread)
)
dtest <- with(
  agaricus.test, xgb.DMatrix(data, label = label, nthread = nthread)
)
evals <- list(eval = dtest)

param <- list(
  max_depth = 2,
  eta = 1,
  nthread = nthread,
  objective = "binary:logistic",
  eval_metric = "auc"
)
bst <- xgb.train(param, dtrain, nrounds = 2, watchlist = evals, verbose = 0)

[david-cortes]

Wouldn't this other PR solve the issue?
#10563

[trivialfis]

It would, we can merge that instead if that's desired. My preference is simpler, the better. I'm less enthusiastic about working around deprecated code.

[david-cortes]

It would, we can merge that instead if that's desired. My preference is simpler, the better. I'm less enthusiastic about working around deprecated code.

I think #10563 would be quite helpful for end users, considering that this will be a major release with lots of deprecations that might cause surprises. It shouldn't be an issue to remove them later, because reverse dependencies would be getting a warning at least.