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

Deprecate parse_bsrn #2466

merged 5 commits into from
Jun 4, 2025

Conversation

kandersolar
Copy link
Member

@kandersolar kandersolar commented Jun 3, 2025

  • 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
@@ -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.

@AdamRJensen AdamRJensen merged commit 15d0911 into pvlib:main Jun 4, 2025
30 checks passed
@kandersolar kandersolar deleted the parse_bsrn branch June 4, 2025 17:18
@kandersolar kandersolar modified the milestones: v0.12.1, v0.13.0 Jun 7, 2025
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