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

Fix error message in variable definition check #2313

Merged
merged 6 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .zenodo.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@
"name": "Kazeroni, RΓ©mi",
"orcid": "0000-0001-7205-9528"
},
{
"affiliation": "GEOMAR, Germany",
"name": "Hohn, David",
"orcid": "0000-0002-5317-1247"
},
{
"affiliation": "DLR, Germany",
"name": "Bauer, Julian"
Expand All @@ -190,6 +195,11 @@
{
"affiliation": "Forschungszentrum Juelich, Germany",
"name": "Benke, Joerg"
},
{
"affiliation": "BSC, Spain",
"name": "Martin-Martinez, Eneko",
"orcid": "0000-0002-9213-7818"
}
],
"description": "ESMValCore: A community tool for pre-processing data from Earth system models in CMIP and running analysis scripts.",
Expand Down
5 changes: 5 additions & 0 deletions CITATION.cff
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ authors:
affiliation: "Forschungszentrum Juelich (FZJ), Germany"
family-names: Benke
given-names: Joerg
-
affiliation: "BSC, Spain"
family-names: Martin-Martinez
given-names: Eneko
orcid: "https://orcid.org/0000-0002-9213-7818"

cff-version: 1.2.0
date-released: 2023-12-19
Expand Down
34 changes: 19 additions & 15 deletions esmvalcore/_recipe/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,21 @@ def diagnostics(diags):
for name, diagnostic in diags.items():
if 'scripts' not in diagnostic:
raise RecipeError(
"Missing scripts section in diagnostic {}".format(name))
f"Missing scripts section in diagnostic '{name}'.")
variable_names = tuple(diagnostic.get('variables', {}))
scripts = diagnostic.get('scripts')
if scripts is None:
scripts = {}
for script_name, script in scripts.items():
if script_name in variable_names:
raise RecipeError(
"Invalid script name {} encountered in diagnostic {}: "
"scripts cannot have the same name as variables.".format(
script_name, name))
f"Invalid script name '{script_name}' encountered "
f"in diagnostic '{name}': scripts cannot have the "
"same name as variables.")
if not script.get('script'):
raise RecipeError(
"No script defined for script {} in diagnostic {}".format(
script_name, name))
f"No script defined for script '{script_name}' in "
f"diagnostic '{name}'.")


def duplicate_datasets(
Expand All @@ -89,27 +89,31 @@ def duplicate_datasets(
"""Check for duplicate datasets."""
if not datasets:
raise RecipeError(
"You have not specified any dataset or additional_dataset groups "
f"for variable {variable_group} in diagnostic {diagnostic}.")
"You have not specified any dataset or additional_dataset "
f"groups for variable '{variable_group}' in diagnostic "
f"'{diagnostic}'.")
checked_datasets_ = []
for dataset in datasets:
if dataset in checked_datasets_:
raise RecipeError(
f"Duplicate dataset {dataset} for variable {variable_group} "
f"in diagnostic {diagnostic}.")
f"Duplicate dataset\n{pformat(dataset)}\nfor variable "
f"'{variable_group}' in diagnostic '{diagnostic}'.")
checked_datasets_.append(dataset)


def variable(var: dict[str, Any], required_keys: Iterable[str]):
def variable(
var: dict[str, Any],
required_keys: Iterable[str],
diagnostic: str,
variable_group: str
) -> None:
"""Check variables as derived from recipe."""
required = set(required_keys)
missing = required - set(var)
if missing:
raise RecipeError(
f"Missing keys {missing} in\n"
f"{pformat(var)}\n"
"for variable {var['variable_group']} in diagnostic "
f"{var['diagnostic']}")
f"Missing keys {missing} in\n{pformat(var)}\nfor variable "
f"'{variable_group}' in diagnostic '{diagnostic}'.")


def _log_data_availability_errors(dataset):
Expand Down
4 changes: 2 additions & 2 deletions esmvalcore/_recipe/recipe_schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ variable:
# TODO: add preprocessor item

diagnostic:
scripts: any(null(), map(include('script')))
scripts: any(null(), map(include('script')), required=False)
additional_datasets: list(include('dataset'), required=False)
title: str(required=False)
description: str(required=False)
Expand All @@ -55,4 +55,4 @@ diagnostic:
variables: map(include('variable'), null(), required=False)

script:
script: str()
script: str(required=False)
4 changes: 4 additions & 0 deletions esmvalcore/_recipe/to_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ def _get_dataset_facets_from_recipe(
recipe_variable: dict[str, Any],
recipe_dataset: dict[str, Any],
profiles: dict[str, Any],
diagnostic_name: str,
session: Session,
) -> tuple[Facets, list[Facets]]:
"""Read the facets for a single dataset definition from the recipe."""
Expand Down Expand Up @@ -286,6 +287,8 @@ def _get_dataset_facets_from_recipe(
'dataset',
'project',
),
diagnostic=diagnostic_name,
variable_group=variable_group
)

preprocessor = facets.get('preprocessor', 'default')
Expand Down Expand Up @@ -329,6 +332,7 @@ def _get_facets_from_recipe(
recipe_variable=recipe_variable,
recipe_dataset=recipe_dataset,
profiles=profiles,
diagnostic_name=diagnostic_name,
session=session,
)

Expand Down
125 changes: 124 additions & 1 deletion tests/integration/recipe/test_recipe.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,80 @@ def get_recipe(tempdir: Path, content: str, session: Session):
return recipe


def test_recipe_missing_scripts(tmp_path, session):
content = dedent("""
datasets:
- dataset: bcc-csm1-1

diagnostics:
diagnostic_name:
variables:
ta:
project: CMIP5
mip: Amon
exp: historical
ensemble: r1i1p1
timerange: 1999/2002
""")
exc_message = ("Missing scripts section in diagnostic 'diagnostic_name'.")
with pytest.raises(RecipeError) as exc:
get_recipe(tmp_path, content, session)
assert str(exc.value) == exc_message


def test_recipe_duplicate_var_script_name(tmp_path, session):
content = dedent("""
datasets:
- dataset: bcc-csm1-1

diagnostics:
diagnostic_name:
variables:
ta:
project: CMIP5
mip: Amon
exp: historical
ensemble: r1i1p1
start_year: 1999
end_year: 2002
scripts:
ta:
script: tmp_path / 'diagnostic.py'
""")
exc_message = ("Invalid script name 'ta' encountered in diagnostic "
"'diagnostic_name': scripts cannot have the same "
"name as variables.")
with pytest.raises(RecipeError) as exc:
get_recipe(tmp_path, content, session)
assert str(exc.value) == exc_message


def test_recipe_no_script(tmp_path, session):
content = dedent("""
datasets:
- dataset: bcc-csm1-1

diagnostics:
diagnostic_name:
variables:
ta:
project: CMIP5
mip: Amon
exp: historical
ensemble: r1i1p1
start_year: 1999
end_year: 2002
scripts:
script_name:
argument: 1
""")
exc_message = ("No script defined for script 'script_name' in "
"diagnostic 'diagnostic_name'.")
with pytest.raises(RecipeError) as exc:
get_recipe(tmp_path, content, session)
assert str(exc.value) == exc_message


def test_recipe_no_datasets(tmp_path, session):
content = dedent("""
diagnostics:
Expand All @@ -181,7 +255,56 @@ def test_recipe_no_datasets(tmp_path, session):
""")
exc_message = ("You have not specified any dataset "
"or additional_dataset groups for variable "
"ta in diagnostic diagnostic_name.")
"'ta' in diagnostic 'diagnostic_name'.")
with pytest.raises(RecipeError) as exc:
get_recipe(tmp_path, content, session)
assert str(exc.value) == exc_message


def test_recipe_duplicated_datasets(tmp_path, session):
content = dedent("""
datasets:
- dataset: bcc-csm1-1
- dataset: bcc-csm1-1

diagnostics:
diagnostic_name:
variables:
ta:
project: CMIP5
mip: Amon
exp: historical
ensemble: r1i1p1
timerange: 1999/2002
scripts: null
""")
exc_message = ("Duplicate dataset\n{'dataset': 'bcc-csm1-1'}\n"
"for variable 'ta' in diagnostic 'diagnostic_name'.")
with pytest.raises(RecipeError) as exc:
get_recipe(tmp_path, content, session)
assert str(exc.value) == exc_message


def test_recipe_var_missing_args(tmp_path, session):
content = dedent("""
datasets:
- dataset: bcc-csm1-1

diagnostics:
diagnostic_name:
variables:
ta:
project: CMIP5
exp: historical
ensemble: r1i1p1
timerange: 1999/2002
scripts: null
""")
exc_message = ("Missing keys {'mip'} in\n{'dataset': 'bcc-csm1-1',"
"\n 'ensemble': 'r1i1p1',\n 'exp': 'historical',\n"
" 'project': 'CMIP5',\n 'short_name': 'ta',\n "
"'timerange': '1999/2002'}\nfor variable 'ta' "
"in diagnostic 'diagnostic_name'.")
with pytest.raises(RecipeError) as exc:
get_recipe(tmp_path, content, session)
assert str(exc.value) == exc_message
Expand Down