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

Improve the exception message when an option within the rootpath section is missing from the user configuration file #2236

Merged
merged 2 commits into from
Nov 13, 2023
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
56 changes: 25 additions & 31 deletions esmvalcore/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ def _get_from_pattern(pattern, date_range_pattern, stem, group):


def _get_start_end_date(
file: str | Path | LocalFile | ESGFFile
) -> tuple[str, str]:
file: str | Path | LocalFile | ESGFFile) -> tuple[str, str]:
"""Get the start and end dates as a string from a file name.

Examples of allowed dates: 1980, 198001, 1980-01, 19801231, 1980-12-31,
Expand Down Expand Up @@ -92,7 +91,6 @@ def _get_start_end_date(
------
ValueError
Start or end date cannot be determined.

"""
if hasattr(file, 'name'): # Path, LocalFile, ESGFFile
stem = Path(file.name).stem
Expand All @@ -114,18 +112,17 @@ def _get_start_end_date(

# Dates can either be delimited by '-', '_', or '_cat_' (the latter for
# CMIP3)
date_range_pattern = (
datetime_pattern + r"[-_](?:cat_)?" + end_datetime_pattern
)
date_range_pattern = (datetime_pattern + r"[-_](?:cat_)?" +
end_datetime_pattern)

# Find dates using the regex
start_date, end_date = _get_from_pattern(datetime_pattern,
date_range_pattern, stem,
'datetime')

# As final resort, try to get the dates from the file contents
if ((start_date is None or end_date is None) and
isinstance(file, (str, Path)) and Path(file).exists()):
if ((start_date is None or end_date is None)
and isinstance(file, (str, Path)) and Path(file).exists()):
logger.debug("Must load file %s for daterange ", file)
cubes = iris.load(file)

Expand All @@ -145,8 +142,7 @@ def _get_start_end_date(
if start_date is None or end_date is None:
raise ValueError(
f"File {file} datetimes do not match a recognized pattern and "
f"time coordinate can not be read from the file"
)
f"time coordinate can not be read from the file")

# Remove potential '-' characters from datetimes
start_date = start_date.replace('-', '')
Expand All @@ -156,12 +152,10 @@ def _get_start_end_date(


def _get_start_end_year(
file: str | Path | LocalFile | ESGFFile
) -> tuple[int, int]:
file: str | Path | LocalFile | ESGFFile) -> tuple[int, int]:
"""Get the start and end year as int from a file name.

See :func:`_get_start_end_date`.

"""
(start_date, end_date) = _get_start_end_date(file)
return (int(start_date[:4]), int(end_date[:4]))
Expand Down Expand Up @@ -224,8 +218,8 @@ def _parse_period(timerange):
start_date = None
end_date = None
time_format = None
datetime_format = (
isodate.DATE_BAS_COMPLETE + 'T' + isodate.TIME_BAS_COMPLETE)
datetime_format = (isodate.DATE_BAS_COMPLETE + 'T' +
isodate.TIME_BAS_COMPLETE)
if timerange.split('/')[0].startswith('P'):
try:
end_date = isodate.parse_datetime(timerange.split('/')[1])
Expand All @@ -246,13 +240,13 @@ def _parse_period(timerange):
end_date = start_date + delta

if time_format == datetime_format:
start_date = str(isodate.datetime_isoformat(
start_date, format=datetime_format))
end_date = str(isodate.datetime_isoformat(
end_date, format=datetime_format))
elif time_format == isodate.DATE_BAS_COMPLETE:
start_date = str(
isodate.date_isoformat(start_date, format=time_format))
isodate.datetime_isoformat(start_date, format=datetime_format))
end_date = str(
isodate.datetime_isoformat(end_date, format=datetime_format))
elif time_format == isodate.DATE_BAS_COMPLETE:
start_date = str(isodate.date_isoformat(start_date,
format=time_format))
end_date = str(isodate.date_isoformat(end_date, format=time_format))

if start_date is None and end_date is None:
Expand Down Expand Up @@ -289,11 +283,11 @@ def _truncate_dates(date, file_date):
def _select_files(filenames, timerange):
"""Select files containing data between a given timerange.

If the timerange is given as a period, the file selection
occurs taking only the years into account.
If the timerange is given as a period, the file selection occurs
taking only the years into account.

Otherwise, the file selection occurs taking into account
the time resolution of the file.
Otherwise, the file selection occurs taking into account the time
resolution of the file.
"""
if '*' in timerange:
# TODO: support * combined with a period
Expand Down Expand Up @@ -414,7 +408,8 @@ def _get_rootpath(project):
key, ', '.join(str(p) for p in nonexistent))
_ROOTPATH_WARNED.add((key, nonexistent))
return rootpath[key]
raise KeyError('default rootpath must be specified in config-user file')
raise KeyError(f'The "{project}" option is missing from the "rootpath" '
'section in the config-user.yml file.')
Comment on lines +411 to +412
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pre-commit hooks made all the changes in this PR, except for this one, which is the one I made :)



def _get_globs(variable):
Expand Down Expand Up @@ -495,8 +490,7 @@ def _get_multiproduct_filename(attributes: dict, preproc_dir: Path) -> Path:
filename_segments = list(dict.fromkeys(filename_segments))

# Add period and extension
filename_segments.append(
f"{attributes['timerange'].replace('/', '-')}.nc")
filename_segments.append(f"{attributes['timerange'].replace('/', '-')}.nc")

outfile = Path(
preproc_dir,
Expand All @@ -517,7 +511,8 @@ def _path2facets(path: Path, drs: str) -> dict[str, str]:
start, end = -len(keys) - 1, -1
values = path.parts[start:end]
facets = {
key: values[idx] for idx, key in enumerate(keys) if "{" not in key
key: values[idx]
for idx, key in enumerate(keys) if "{" not in key
}

if len(facets) != len(keys):
Expand All @@ -532,8 +527,7 @@ def _path2facets(path: Path, drs: str) -> dict[str, str]:


def _filter_versions_called_latest(
files: list['LocalFile'],
) -> list['LocalFile']:
files: list['LocalFile']) -> list['LocalFile']:
"""Filter out versions called 'latest' if they are duplicates.

On compute clusters it is usual to have a symbolic link to the
Expand Down
28 changes: 28 additions & 0 deletions tests/unit/local/test_get_rootpath.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
"""Tests for ``_get_rootpath`` in ``esmvalcore.local``."""
from unittest import mock

import pytest

from esmvalcore import local


@mock.patch("os.path.exists")
def test_get_rootpath_exists(mexists):
mexists.return_value = True
cfg = {"rootpath": {"CMIP5": ["/path1"], "CMIP6": ["/path2"]}}
project = "CMIP5"
with mock.patch.dict(local.CFG, cfg):
output = local._get_rootpath(project)
# 'output' is a list containing a PosixPath:
assert str(output[0]) == cfg["rootpath"][project][0]


@mock.patch("os.path.exists")
def test_get_rootpath_does_not_exist(mexists):
mexists.return_value = False
cfg = {"rootpath": {"CMIP5": ["path1"], "CMIP6": ["path2"]}}
project = "OBS"
with mock.patch.dict(local.CFG, cfg):
msg = rf"The \"{project}\" option is missing.*"
with pytest.raises(KeyError, match=msg):
local._get_rootpath(project)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, you made these changes too, unless precommit hooks now write unit tests too (which would be fab) 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🀣 🀣 🀣 I added these after I made that comment! 😝