Skip to content

Clarify output variable mapping in calcparams_ functions #2405

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 6 commits into from
Mar 11, 2025

Conversation

MaxJackson
Copy link
Contributor

@MaxJackson MaxJackson commented Mar 11, 2025

  • Closes #716
  • I am familiar with the contributing guidelines
  • 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.

The functions pvsystem.calcparams_desoto, pvsystem.calcparams_cec and pvsystem.calcparams_pvsyst return variables which are named/labeled differently than the variables in the functions themselves. This PR adds inline comments where the return variables are initialized to help clarify exactly what calculated values are being returned.

MaxJackson and others added 3 commits March 11, 2025 11:39
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
@MaxJackson
Copy link
Contributor Author

@cwhanse I've merged your suggestions into my PR branch!

@cwhanse
Copy link
Member

cwhanse commented Mar 11, 2025

@MaxJackson can you add yourself as a contributor to the whatsnew file? In docs/sphinx/source/whatsnew/v0.11.3.rst I don't think we need a note about this change.

@cwhanse
Copy link
Member

cwhanse commented Mar 11, 2025

Two trailing whitespaces to remove also :)

@MaxJackson
Copy link
Contributor Author

@cwhanse done!

Copy link
Member

@cwhanse cwhanse left a comment

Choose a reason for hiding this comment

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

thanks @MaxJackson

And for fixing the typo in the whatsnew file

@cwhanse cwhanse added this to the v0.11.3 milestone Mar 11, 2025
@cwhanse cwhanse merged commit 4b113ab into pvlib:main Mar 11, 2025
31 checks passed
@kandersolar kandersolar modified the milestones: v0.11.3, v0.12.0 Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants