Skip to content

Add dataset option to get_era5 #2780

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.

Sign up for GitHub

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

Open
AdamRJensen wants to merge 8 commits into
base: main
Choose a base branch
from
+30 −6
Open

Add dataset option to get_era5 #2780

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a997b6b
Add dataset option to get_era5
AdamRJensen Jun 10, 2026
7de716c
Update era5-land test to use np.isclose
AdamRJensen Jun 10, 2026
97f1ee4
Add numpy import to test_era5.py
AdamRJensen Jun 10, 2026
fce831e
Implement suggested changes from review
AdamRJensen Jun 10, 2026
3dfbac5
Add whatsnew entry
AdamRJensen Jun 10, 2026
c7eaddd
Fix linter
AdamRJensen Jun 10, 2026
0355369
Apply suggestions from code review
AdamRJensen Jun 11, 2026
0faccdc
Merge branch 'main' into add-era5-land
AdamRJensen Jun 11, 2026
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
7 changes: 6 additions & 1 deletion pvlib/iotools/era5.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def _m_to_cm(m):


def get_era5(latitude, longitude, start, end, variables, api_key,
dataset="reanalysis-era5-single-levels-timeseries",
map_variables=True, timeout=60,
url='https://cds.climate.copernicus.eu/api/retrieve/v1/'):
"""
Expand All @@ -84,6 +85,10 @@ def get_era5(latitude, longitude, start, end, variables, api_key,
See [1]_ for additional options.
api_key : str
ECMWF CDS API key.
dataset : str, default "reanalysis-era5-single-levels-timeseries"
Comment thread
AdamRJensen marked this conversation as resolved.
Outdated
The dataset to query. May be either
"reanalysis-era5-single-levels-timeseries" or
"reanalysis-era5-land-timeseries".
Comment thread
AdamRJensen marked this conversation as resolved.
Outdated
Comment thread
AdamRJensen marked this conversation as resolved.
Outdated
map_variables : bool, default True
When true, renames columns of the DataFrame to pvlib variable names
where applicable. Also converts units of some variables. See variable
Expand Down Expand Up @@ -137,7 +142,7 @@ def _to_utc_dt_notz(dt):
"data_format": "csv"
}
}
slug = "processes/reanalysis-era5-single-levels-timeseries/execution"
slug = f"processes/{dataset}/execution"

@echedey-ls echedey-ls Jun 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This means that if somebody copies the dataset parameter with a typo (e.g., an space) the error is harder to spot. I suggest to add a guardrail right before (if dataset not in {"...", "..."}. In addition to that, the new function signature is not backwards-compatible in those calls that provided positional parameters up to map_variables (inclusive). The guardrail would provide a meaningful message in this case, where dataset=True/False.

@AdamRJensen AdamRJensen Jun 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, I like the idea, but overall I'm against it. My reason is that if a new compatible dataset is added, then users would not be able to use the function. For example, I was not aware that the ERA5-Land had become an option.

Indeed, it is not backward compatible, this is why I am lobbying for making most input parameters keyword only.

@echedey-ls echedey-ls Jun 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I admit to not suggesting that to avoid being pedantic 😆

I don't think it's the right time for this release, but I remember PyVista has [at least in v0.46.something] a deprecation decorator that warns against the use of positional arguments (while it also allowed passing some). Just in case it's helpful in the future if iotools API was to be standardized.

response = requests.post(url + slug, json=params, headers=headers,
timeout=timeout)
submission_response = response.json()
Expand Down
13 changes: 12 additions & 1 deletion tests/iotools/test_era5.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,20 @@
def test_get_era5(params, expected):
df, meta = pvlib.iotools.get_era5(**params)
pd.testing.assert_frame_equal(df, expected, check_freq=False, atol=0.1)
assert np.isclose(meta['longitude'], -80.0)

Check failure on line 46 in tests/iotools/test_era5.py

View workflow job for this annotation

GitHub Actions / flake8-linter 

F821 undefined name 'np'
assert np.isclose(meta['latitude'], 40.0)

Check failure on line 47 in tests/iotools/test_era5.py

View workflow job for this annotation

GitHub Actions / flake8-linter 

F821 undefined name 'np'
assert isinstance(meta['jobID'], str)


@requires_ecmwf_credentials
@pytest.mark.remote_data
@pytest.mark.flaky(reruns=RERUNS, reruns_delay=RERUNS_DELAY)
def test_get_era5_land(params, expected):
params['dataset'] = "reanalysis-era5-land-timeseries"
df, meta = pvlib.iotools.get_era5(**params)
assert meta['longitude'] == -80.0
assert meta['latitude'] == 40.0
assert isinstance(meta['jobID'], str)
assert pd.testing.assert_index_equal(df.index, expected.index)


@requires_ecmwf_credentials
Expand Down
Loading