YAML config file management similar to the config package

[wlandau]

Prework

• Read and agree to the code of conduct and contributing guidelines.
• If there is already a relevant issue, whether open or closed, comment on the existing thread instead of posting a new issue.
• [n/a] For any problems you identify, post a minimal reproducible example like this one so the maintainer can troubleshoot. A reproducible example is:
  • [n/a] Runnable: post enough R code and data so any onlooker can create the error on their own computer.
  • [n/a] Minimal: reduce runtime wherever possible and remove complicated details that are irrelevant to the issue at hand.
  • [n/a] Readable: format your code according to the tidyverse style guide.

Description

The config package would make the existing _targets.yaml/tar_config_set()/tar_config_get() system more powerful. I am particularly interested in inheritance. This would make it easier to create sub-projects.

[wlandau]

Related: #622

[wlandau]

On second thought, the config package is not a great fit because it requires supporting multiple configurations in a single YAML file. That would break the format of _targets.yaml.

[wlandau]

And manually implementing inheritance without the config package seems like overkill.

[wlandau]

#622 (comment) and #622 (comment) are relevant and make this issue worth considering.

[wlandau]

Reasons in favor:

• config is the state of the art, a reliable and ubiquitous standard for key/value pairs.
• Inheritance would make it easier to manage multiple projects.
• Only need to define a single config file. No need for one config file per sub-project.

Reasons against:

• The refactor would be painful for both myself and users if we want back-compatibility with the current _targets.yaml format.
• This usage of the config package would make the tar_config_set()/tar_config_get() abstraction leakier. Users would need to understand the concepts of multiple configurations and inheritance explained in config. The targets layer on top does not abstract this part away cleanly, which could cause confusion for users.
• targets configuration is usually simple and may not always require everything in the config package.
• With Consider setting the YAML config file with an environment variable #622, it will be possible to define a temporary config file for a project, which could go in the setup chunk of a Target Markdown document. This should cover most use cases without having to

Sys.setenv(TAR_CONFIG = tempfile())
tar_config_set(
  script = "alternate_script.R",
  store = "alterante_store"
)

So I will table this issue for now and revisit it if there is a specific need.

[wlandau]

This usage of the config package would make the tar_config_set()/tar_config_get() abstraction leakier.

On second thought, not really. Configuration spaces like "default" are for individual projects, and new users generally have prior intuition about that.

[wlandau]

Notes:

• Env vars: TAR_CONFIG to set the config file and TAR_PROJECT to set the project within the config file. These env vars have counterparts R_CONFIG_FILE and R_CONFIG_ACTIVE, respectively, in the config package.
• tar_config_set() should do the conversion from the old YAML format automatically and write a message. tar_config_get() should stay compatible with both the new and old formats, and it should throw a deprecation advertising the new format and how to opt in.
• tar_option_set() needs a new inherits argument to enable inheritance among projects.

[wlandau]

After some tinkering, I do plan to implement config-like functionality (e.g. inheritance) but not actually use the config package itself. Reasons:

1. config::get() reads the YAML file, and targets needs to manually read the YAML file once before that in order to check for the deprecated format. So if we went with config, targets would need to read the YAML file twice.
2. config::get() requires a configuration called "default", and it must not be empty. Otherwise, config throws an error.
3. config assumes all projects inherit from the default project, and this is not desirable in targets.
4. config assumes all fields must be explicitly populated. targets has system defaults, and I do not want to make the user recapitulate these.
5. Project inheritance is actually pretty simple to implement. Even checking for circular inheritance should is not too hard using a hash table and a little recursion.

[wlandau]

Almost done with the implementation, just need to populate the tests below:

targets/tests/testthat/test-tar_config_set.R

Lines 198 to 232 in b1f9c30

	tar_test("single-project format still works", {
	stop("Remember to implement!")
	})
	tar_test("single-project converted to multi-project", {
	stop("Remember to implement!")
	})
	tar_test("project switching/setting with project arg", {
	stop("Remember to implement!")
	})
	tar_test("project switching/setting with TAR_PROJECT", {
	stop("Remember to implement!")
	})
	tar_test("correct project inheritance", {
	stop("Remember to implement!")
	})
	tar_test("inherit from project without field", {
	stop("Remember to implement!")
	})
	tar_test("inherit from nonexistent project", {
	stop("Remember to implement!")
	})
	tar_test("nontrivial circular project inheritance", {
	stop("Remember to implement!")
	})
	tar_test("project inherits from itself", {
	stop("Remember to implement!")
	})

targets/tests/testthat/test-tar_config_unset.R

Lines 15 to 21 in b1f9c30

	tar_test("tar_config_unset() unsets from correct project", {
	stop("Remember to implement!")
	})
	tar_test("tar_config_unset() converts to multi-project format", {
	stop("Remember to implement!")
	})