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

Improvements for read_optional_key implementation #284

Open
SeanBryan51 opened this issue Apr 24, 2024 · 0 comments
Open

Improvements for read_optional_key implementation #284

SeanBryan51 opened this issue Apr 24, 2024 · 0 comments
Labels
enhancement New feature or request priority:low Low priority Issues that do not need to be addressed in the near future. testing Add unit/integration tests

Comments

@SeanBryan51
Copy link
Collaborator

The read_optional_key function was added in PR #238:

def read_optional_key(config: dict):
"""Fills all optional keys in config if not already defined.
The default values for most optional keys are loaded from `internal.py`
Note: We need to ensure that the `name` key for realisations exists for
other modules, but it doesn't have a default value. So we set it to
`None` by default.
Parameters
----------
config : dict
The configuration file with with/without optional keys
"""
if "project" not in config:
config["project"] = os.environ.get("PROJECT", None)
if "realisations" in config:
for r in config["realisations"]:
r["name"] = r.get("name")
config["science_configurations"] = config.get(
"science_configurations", internal.DEFAULT_SCIENCE_CONFIGURATIONS
)
# Default values for spatial
config["spatial"] = config.get("spatial", {})
config["spatial"]["met_forcings"] = config["spatial"].get(
"met_forcings", internal.SPATIAL_DEFAULT_MET_FORCINGS
)
config["spatial"]["payu"] = config["spatial"].get("payu", {})
config["spatial"]["payu"]["config"] = config["spatial"]["payu"].get("config", {})
config["spatial"]["payu"]["args"] = config["spatial"]["payu"].get("args")
# Default values for fluxsite
config["fluxsite"] = config.get("fluxsite", {})
config["fluxsite"]["multiprocess"] = config["fluxsite"].get(
"multiprocess", internal.FLUXSITE_DEFAULT_MULTIPROCESS
)
config["fluxsite"]["experiment"] = config["fluxsite"].get(
"experiment", internal.FLUXSITE_DEFAULT_EXPERIMENT
)
config["fluxsite"]["pbs"] = internal.FLUXSITE_DEFAULT_PBS | config["fluxsite"].get(
"pbs", {}
)

Instead of hard coding default values for each field within the function, we can use the schema itself to set default values for missing fields in the document. The cerberus python package provides normalization rules which support this functionality. This approach would also lets us change default values for specific fields in the future without breaking the test suite.

@SeanBryan51 SeanBryan51 added enhancement New feature or request testing Add unit/integration tests labels Apr 24, 2024
@ccarouge ccarouge added the priority:low Low priority Issues that do not need to be addressed in the near future. label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority:low Low priority Issues that do not need to be addressed in the near future. testing Add unit/integration tests
Projects
None yet
Development

No branches or pull requests

2 participants