Repository navigation
Villalva Algorithm Pull Request - #2878
JFrederico2022 wants to merge 27 commits into
Conversation
Hey @JFrederico2022! 🎉Thanks for opening your first pull request! We appreciate your If AI is used for any portion of this PR, you must vet the content Finally, be sure the PR description includes the PR |
|
@JFrederico2022 pvlib should get only the functions, not the notebook. It has been a while since we discussed that: #2754 (comment) "In pvlib, a calcparams_villalva function would go into pvlib.pvsystem. Code for fitting would go into pvlib.ivtools.sdm in a new module villalva.py" If you want, you can convert the notebook to a python script and add to the Example Gallery. Just start the script name with "plot_" and use docstrings where you want text to appear. |
|
Hello, @cwhanse I updated the docs. Please, let me know if they are all right. Best regards, |
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
|
Hi @JFrederico2022, great initiative! In my opinion, I think it would be nice for your PR to have a corresponding issue which includes a description of what you are adding to pvlib (e.g., what do you see is missing that a PR would add and include sources). That way reviewers and curious people can have a quick outlook of what's happening. |
The issue exists (#2754) but, @JFrederico2022, please tag this in the PR template |
|
Remember to list the new functions in https://github.com/pvlib/pvlib-python/blob/main/docs/sphinx/source/reference/pv_modeling/sdm.rst?plain=1 |
There was a problem hiding this comment.
I have suggested a few basic docstring revisions. You can implement changes recommended by reviewers either by committing them individually or, under the Files changed tab, adding the suggestions to a batch and committing them all at once.
Happy to look more in depth at the core implementation in the coming days
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Co-authored-by: Rajiv Daxini <143435106+RDaxini@users.noreply.github.com>
Updated references for M. G. Villalva's work in the documentation.
Added noqa comments to suppress E231 warnings for print statements and labels.
Added fit_villalva function to the list of fitting functions.
docs/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.