Skip to content
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

Correct racking_model default value, add freestanding option #2232

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

RDaxini
Copy link
Contributor

@RDaxini RDaxini commented Sep 28, 2024

Four primary changes:

  • Updated docstring value for racking_model from open_rack to None (comment)
  • Added freestanding option for racking_model
  • Updated comment to remove specific reference to SAPM (since pvsyst or sapm could be used depending on the value of racking_model, if I have understood correctly)
  • Added None as default value for module_height

Other changes:
Basic formatting and other edits (e.g. adding units, swapping apostrophes for backticks), removed unused package

@RDaxini
Copy link
Contributor Author

RDaxini commented Sep 28, 2024

I have limited these changes to PVSystem and FixedMount only as these are the two classes mentioned in #1942
I could make further edits, e.g. more formatting, update/add default values for the same (and other) parameters in the other docstrings, etc. either in this PR if recommended, or in a follow up PR if this would become too cluttered (please advise)

@RDaxini RDaxini marked this pull request as ready for review September 28, 2024 21:07
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.

I agree with limiting the formatting fixes to the functions/classes touched by the primary purpose of the PR.

pvlib/pvsystem.py Outdated Show resolved Hide resolved
pvlib/pvsystem.py Outdated Show resolved Hide resolved
@cwhanse cwhanse added this to the v0.11.2 milestone Sep 29, 2024
pvlib/pvsystem.py Outdated Show resolved Hide resolved
pvlib/pvsystem.py Outdated Show resolved Hide resolved
pvlib/pvsystem.py Outdated Show resolved Hide resolved
pvlib/pvsystem.py Outdated Show resolved Hide resolved
pvlib/pvsystem.py Outdated Show resolved Hide resolved
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