PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
202 stars 235 forks source link

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

Open Aariq opened 1 year ago

Aariq commented 1 year ago

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: https://github.com/PecanProject/pecan/issues/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.

github-actions[bot] commented 10 months ago

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