Skip to content

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