Add support for iam.schlick to ModelChain - #2832
Conversation
|
@cbcrespo Given Anton's comment here I think we should also add a warning to the function itself (an admonition) stating something along the lines of:
|
RDaxini
left a comment
There was a problem hiding this comment.
LGTM, just a note on pvsystem.py and the whatnsew. #2832 (comment) should also be addressed and then I think this PR is ready to go.
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
kandersolar
left a comment
There was a problem hiding this comment.
I think some tests for the PVSystem and Array methods would be good too. There should be existing tests that you can easily hook into like you did with the ModelChain tests.
One other thing: of course we can't infer schlick from the parameters (since it takes no parameters), but I suppose a comment in the inference function along those lines would be nice. Something here indicating that schlick is omitted on purpose:
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
|
Linter failure is on lines not touched by this PR |
iam.schlicktoModelChain#2828Updates entries indocs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor 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`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.Currently, all direct IAM models available in
pvlib.iamare supported byModelChainexcept forschlick. This PR adds support forschlick.