Skip to content

Commit

Permalink
[R-package] fix handling of duplicate parameters (fixes #4521) (#4914)
Browse files Browse the repository at this point in the history
* [R-package] fix handling of duplicate parameters (fixes #4521)

* add gsub stuff back in for now
  • Loading branch information
jameslamb authored Dec 29, 2021
1 parent f9053ab commit ab78829
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
9 changes: 4 additions & 5 deletions R-package/R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,19 @@ lgb.params2str <- function(params) {
stop("params must be a list")
}

# Split parameter names
names(params) <- gsub("\\.", "_", names(params))

param_names <- names(params)
ret <- list()

# Perform key value join
for (key in names(params)) {
for (i in seq_along(params)) {

# If a parameter has multiple values, join those values together with commas.
# trimws() is necessary because format() will pad to make strings the same width
val <- paste0(
trimws(
format(
x = params[[key]]
x = unname(params[[i]])
, scientific = FALSE
)
)
Expand All @@ -47,7 +46,7 @@ lgb.params2str <- function(params) {
if (nchar(val) <= 0L) next # Skip join

# Join key value
pair <- paste0(c(key, val), collapse = "=")
pair <- paste0(c(param_names[[i]], val), collapse = "=")
ret <- c(ret, pair)

}
Expand Down
11 changes: 11 additions & 0 deletions R-package/tests/testthat/test_utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ test_that("lgb.params2str() works as expected for a key in params with multiple
)
})

test_that("lgb.params2str() passes through duplicated params", {
out_str <- lgb.params2str(
params = list(
objective = "regression"
, bagging_fraction = 0.8
, bagging_fraction = 0.5
)
)
expect_equal(out_str, "objective=regression bagging_fraction=0.8 bagging_fraction=0.5")
})

context("lgb.check.eval")

test_that("lgb.check.eval works as expected with no metric", {
Expand Down

0 comments on commit ab78829

Please sign in to comment.