Skip to content

Deprecate parse_bsrn #2466

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

Merged
merged 5 commits into from
Jun 4, 2025
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
3 changes: 2 additions & 1 deletion docs/sphinx/source/whatsnew/v0.12.1.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ Breaking Changes
Deprecations
~~~~~~~~~~~~
* The following ``parse_`` functions in :py:mod:`pvlib.iotools` are deprecated,
with the corresponding ``read_`` functions taking their place: (:issue:`2444`, :pull:`2458`)
with the corresponding ``read_`` functions taking their place: (:issue:`2444`, :pull:`2458`, :pull:`2466`)

- :py:func:`~pvlib.iotools.parse_psm3`
- :py:func:`~pvlib.iotools.parse_cams`
- :py:func:`~pvlib.iotools.parse_bsrn`

* The ``server`` parameter in :py:func:`~pvlib.iotools.get_cams` has been renamed
to ``url`` to be consistent with the other iotools.
Expand Down
21 changes: 14 additions & 7 deletions pvlib/iotools/bsrn.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import io
import os

from pvlib.tools import _file_context_manager
from pvlib._deprecation import deprecated

BSRN_FTP_URL = "ftp.bsrn.awi.de"

BSRN_LR0100_COL_SPECS = [(0, 3), (4, 9), (10, 16), (16, 22), (22, 27),
Expand Down Expand Up @@ -136,7 +139,7 @@ def get_bsrn(station, start, end, username, password,

See Also
--------
pvlib.iotools.read_bsrn, pvlib.iotools.parse_bsrn
pvlib.iotools.read_bsrn

References
----------
Expand Down Expand Up @@ -191,7 +194,7 @@ def get_bsrn(station, start, end, username, password,
bio.seek(0) # reset buffer to start of file
gzip_file = io.TextIOWrapper(gzip.GzipFile(fileobj=bio),
encoding='latin1')
dfi, metadata = parse_bsrn(gzip_file, logical_records)
dfi, metadata = _parse_bsrn(gzip_file, logical_records)
dfs.append(dfi)
# FTP client raises an error if the file does not exist on server
except ftplib.error_perm as e:
Expand All @@ -217,7 +220,7 @@ def get_bsrn(station, start, end, username, password,
return data, metadata


def parse_bsrn(fbuf, logical_records=('0100',)):
def _parse_bsrn(fbuf, logical_records=('0100',)):
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason that we aren't putting this content in the read function and completely get rid of the parse function? It is a bit of a hassle, but long term probably worth it it seems.

Copy link
Member Author

Choose a reason for hiding this comment

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

I took the easy route here and in #2467 for expediency in getting the deprecations in place. Let's open an issue about refactoring these private functions?

Copy link
Member

Choose a reason for hiding this comment

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

After posting that comment, I realized that might be the case. Fine by me.

"""
Parse a file-like buffer of a BSRN station-to-archive file.

Expand Down Expand Up @@ -382,7 +385,7 @@ def read_bsrn(filename, logical_records=('0100',)):
Parameters
----------
filename: str or path-like
Name or path of a BSRN station-to-archive data file
Name, path, or in-memory buffer of a BSRN station-to-archive data file
logical_records: list or tuple, default: ('0100',)
List of the logical records (LR) to parse. Options include: '0100',
'0300', and '0500'.
Expand Down Expand Up @@ -439,7 +442,7 @@ def read_bsrn(filename, logical_records=('0100',)):

See Also
--------
pvlib.iotools.parse_bsrn, pvlib.iotools.get_bsrn
pvlib.iotools.get_bsrn

References
----------
Expand All @@ -457,7 +460,11 @@ def read_bsrn(filename, logical_records=('0100',)):
if str(filename).endswith('.gz'): # check if file is a gzipped (.gz) file
open_func, mode = gzip.open, 'rt'
else:
open_func, mode = open, 'r'
open_func, mode = _file_context_manager, 'r'
with open_func(filename, mode) as f:
content = parse_bsrn(f, logical_records)
content = _parse_bsrn(f, logical_records)
return content


parse_bsrn = deprecated(since="0.13.0", name="parse_bsrn",
alternative="read_bsrn")(read_bsrn)
21 changes: 20 additions & 1 deletion tests/iotools/test_bsrn.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest
import os
import tempfile
from pvlib.iotools import read_bsrn, get_bsrn
from pvlib.iotools import read_bsrn, get_bsrn, parse_bsrn
from tests.conftest import (
TESTS_DATA_DIR,
RERUNS,
Expand All @@ -15,6 +15,8 @@
requires_bsrn_credentials,
)

from pvlib._deprecation import pvlibDeprecationWarning


@pytest.fixture(scope="module")
def bsrn_credentials():
Expand All @@ -33,6 +35,12 @@ def expected_index():
tz='UTC')


def test_parse_bsrn_deprecated():
with pytest.warns(pvlibDeprecationWarning, match='Use read_bsrn instead'):
with open(TESTS_DATA_DIR / 'bsrn-lr0100-pay0616.dat') as fbuf:
data, metadata = parse_bsrn(fbuf)


@pytest.mark.parametrize('testfile', [
('bsrn-pay0616.dat.gz'),
('bsrn-lr0100-pay0616.dat'),
Expand All @@ -47,6 +55,17 @@ def test_read_bsrn(testfile, expected_index):
assert 'relative_humidity' in data.columns


def test_read_bsrn_buffer(expected_index):
with open(TESTS_DATA_DIR / 'bsrn-lr0100-pay0616.dat') as fbuf:
data, metadata = read_bsrn(fbuf)
assert_index_equal(expected_index, data.index)
assert 'ghi' in data.columns
assert 'dni_std' in data.columns
assert 'dhi_min' in data.columns
assert 'lwd_max' in data.columns
assert 'relative_humidity' in data.columns


def test_read_bsrn_logical_records(expected_index):
# Test if logical records 0300 and 0500 are correct parsed
# and that 0100 is not passed when not specified
Expand Down
Loading