search

mlr-org / mlr3tuning

Hyperparameter optimization package of the mlr3 ecosystem
https://mlr3tuning.mlr-org.com/
GNU Lesser General Public License v3.0
55 stars 5 forks source link

library(mlr3pipelines) makes it required for tests, as opposed to the skip_if_not_installed() usage elsewhere #470

Closed MichaelChirico closed 2 days ago

MichaelChirico commented 3 days ago

These two lines are contradictory:

https://github.com/mlr-org/mlr3tuning/blob/4b6c958659b20d1c96cc7b7e46117b97df922d33/tests/testthat/helper.R#L4

https://github.com/mlr-org/mlr3tuning/blob/4b6c958659b20d1c96cc7b7e46117b97df922d33/tests/testthat/test_Tuner.R#L72

Because helper.R is always sourced before running a test_ file.

IMO if {mlr3pipelines} is required for the tests, it is better to put the library() call in tests/testthat.R -- this is the first place a reader will look for test requirements.

If it's indeed optional, I would remove the library() call in helper.R.

be-marc commented 2 days ago

Thanks for raising the issue. Fixed in #471