merge_lists does not work properly

ShixiangWang commented 2 years ago

The result config should be set to expression(Sys.getenv("coco.dev_database")) in dev config instead of the setting in default config.

Browse[3]> dev_config = do_get(config)
Browse[3]> dput(default_config)
list(golem_name = "coco", golem_version = "0.0.0.9000", coco.databases = list(
    structure(expression(coco::app_sys("cohorts")), srcfile = <environment>, wholeSrcref = structure(c(1L, 
    0L, 2L, 0L, 0L, 0L, 1L, 2L), srcfile = <environment>, class = "srcref")), 
    "~/.coco/cohorts", "An invalid path"), coco.cache_size = 2L, 
    coco.cache_dir = structure(expression(file.path(tempdir(TRUE), 
        "coco")), srcfile = <environment>, wholeSrcref = structure(c(1L, 
    0L, 2L, 0L, 0L, 0L, 1L, 2L), srcfile = <environment>, class = "srcref")), 
    app_prod = FALSE)
Browse[3]> dput(dev_config)
list(app_prod = FALSE, golem_wd = structure(expression(here::here()), srcfile = <environment>, wholeSrcref = structure(c(1L, 
0L, 2L, 0L, 0L, 0L, 1L, 2L), srcfile = <environment>, class = "srcref")), 
    coco.databases = list(structure(expression(Sys.getenv("coco.dev_database")), srcfile = <environment>, wholeSrcref = structure(c(1L, 
    0L, 2L, 0L, 0L, 0L, 1L, 2L), srcfile = <environment>, class = "srcref"))), 
    coco.cache_dir = "~/.coco_cache")
Browse[3]> merge_lists(default_config, dev_config)
$golem_name
[1] "coco"

$golem_version
[1] "0.0.0.9000"

$coco.databases
$coco.databases[[1]]
expression(coco::app_sys("cohorts"))

$coco.databases[[2]]
[1] "~/.coco/cohorts"

$coco.databases[[3]]
[1] "An invalid path"

$coco.cache_size
[1] 2

$app_prod
[1] FALSE

$golem_wd
expression(here::here())

$coco.cache_dir
[1] "~/.coco_cache"

Browse[3]> default_config
$golem_name
[1] "coco"

$golem_version
[1] "0.0.0.9000"

$coco.databases
$coco.databases[[1]]
expression(coco::app_sys("cohorts"))

$coco.databases[[2]]
[1] "~/.coco/cohorts"

$coco.databases[[3]]
[1] "An invalid path"

$coco.cache_size
[1] 2

$coco.cache_dir
expression(file.path(tempdir(TRUE), "coco"))

$app_prod
[1] FALSE

Browse[3]> dev_config
$app_prod
[1] FALSE

$golem_wd
expression(here::here())

$coco.databases
$coco.databases[[1]]
expression(Sys.getenv("coco.dev_database"))

$coco.cache_dir
[1] "~/.coco_cache"

I debug in deep and find out that merge_lists() use recursive approach at default (can not be modified outside), when merge two lists without name, the following situation happens, 4 cannot replace 1.

> config:::merge_lists(list(1, 2, 3), list(4))
[[1]]
[1] 1

[[2]]
[1] 2

[[3]]
[1] 3

> config:::merge_lists(list(a = 1, b = 2, c = 3), list(a = 4))
$b
[1] 2

$c
[1] 3

$a
[1] 4

ShixiangWang commented 2 years ago

I go further check and generate an example, get some strange results:

test.yml

default:
  databases:
    - !expr c("abc")
    - ~/abc
dev:
  databases:
    - /opt/abc
    - !expr Sys.getenv("R")

Don't work at default

> config::get("databases", "dev", file = "~/Downloads/test.yml")
[[1]]
[1] "abc"

[[2]]
[1] "~/abc"

However, if I remove any rows with !expr, it works. I find that if insert a row with !expr, it will be parsed as an individual list, while without, a character vector is generated.

andrie commented 2 years ago

If you use only named elements this should work as expected:

default:
  databases:
    db1: !expr c("abc")
    db2: ~/abc
dev:
  databases:
    db1: /opt/abc
    db2: !expr Sys.getenv("R")

ShixiangWang commented 2 years ago

@andrie Ohh, thanks for your illustration.

psychelzh commented 1 year ago

Is there any workarounds when the elements are not named?

rstudio / config

merge_lists does not work properly #42