Skip to content

Deprecate parse_bsrn#2466

Merged
AdamRJensen merged 5 commits into
pvlib:mainfrom
kandersolar:parse_bsrn
Jun 4, 2025
Merged

Deprecate parse_bsrn#2466
AdamRJensen merged 5 commits into
pvlib:mainfrom
kandersolar:parse_bsrn

Conversation

@kandersolar

@kandersolar kandersolar commented Jun 3, 2025

Copy link
Copy Markdown
Member
  • Towards Consider removing the parse_ iotools functions #2444
  • I am familiar with the contributing guidelines
  • Tests added
  • Updates entries in docs/sphinx/source/reference for API changes.
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

@kandersolar kandersolar added this to the v0.12.1 milestone Jun 3, 2025
@kandersolar kandersolar added api io remote-data triggers --remote-data pytests labels Jun 3, 2025
@kandersolar kandersolar changed the title Deprecate _parse_bsrn Deprecate parse_bsrn Jun 3, 2025
@kandersolar
kandersolar requested a review from AdamRJensen June 3, 2025 16:31
@kandersolar
kandersolar marked this pull request as ready for review June 3, 2025 16:31
Comment thread pvlib/iotools/bsrn.py


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

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.

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
Copy Markdown
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
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api io remote-data triggers --remote-data pytests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants