Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use options() for project-level settings like write #3064

Open
Aariq opened this issue Nov 10, 2022 · 1 comment
Open

Use options() for project-level settings like write #3064

Aariq opened this issue Nov 10, 2022 · 1 comment

Comments

@Aariq
Copy link
Collaborator

Aariq commented Nov 10, 2022

Description

I'll use settings$database$bety$write as an example, but this likely applies to other settings. Currently, the settings object, or pieces of it, get passed down to functions to determine their behavior. But sometimes we forget to add all the arguments or pass all the pieces of settings. For example: #2968.

When a new function is added to PEcAn that needs to know whether to write to BETY, we need to rely on passing it the settings object. This makes PEcAn less modular and harder to develop and maintain.

Proposed Solution

An alternative approach is to use options(). As an example, let's use an option called "pecan.write_bety". This could get set by read.settings() based on the <write> tag or by a user including options(pecan.write_bety = FALSE) at the top of a script or in a project-level .Rprofile (we'll need to figure out which one of those should "win"). Then rather than passing settings around, the relevant PEcAn.DB functions would getOption("pecan.write_bety", default = FALSE) before deciding whether to write to BETY or not.

This change would make all write arguments to functions obsolete and they would need to be deprecated appropriately (e.g. if supplied, they'd set the option and print a warning.)

It would also mean that any new functions that use PEcAn.DB would not need to include a write argument or worry about that portion of settings. This paradigm could be extended to other parts of pecan.xml/settings that are more "config-like" than they are defining individual runs.

Alternatives Considered

The more "config-like" parts of pecan.xml could be split out into a proper config file (a typical pattern used by other R packages would be something like _pecan.yml, with an environment variable to set a non-default file name) and then functions would just read config settings directly from _pecan.yml to figure out if they should write or not.

Copy link

This issue is stale because it has been open 365 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant