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

Implement pvlib.irradiance.louche decomposition model #1705

Merged
merged 17 commits into from May 30, 2023

Conversation

Lakshyadevelops
Copy link
Contributor

@Lakshyadevelops Lakshyadevelops commented Mar 23, 2023

  • Fulfills pvl_louche from bring pvlib-python up to date with PVLIB-MATLAB 1.2 and 1.3 #2
  • 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.

@Lakshyadevelops
Copy link
Contributor Author

Lakshyadevelops commented Mar 23, 2023

Confused in tests, shall I take subset of tests from 723650TY.csv which are testing all possibilities or some other source?
@pvlib

@Lakshyadevelops Lakshyadevelops marked this pull request as ready for review March 27, 2023 21:44
@Lakshyadevelops
Copy link
Contributor Author

Hey guys please have a look over the PR and let me know of any changes.
@cwhanse

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'll start the CI for this one.

pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
@cwhanse cwhanse added this to the 0.9.6 milestone Mar 31, 2023
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
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.

A few formatting adjustment. Thanks @Lakshyadevelops

@Lakshyadevelops
Copy link
Contributor Author

Thanks for the guidance @cwhanse.
Is it worth implementing other matlab functions now?

Copy link
Member

@kandersolar kandersolar left a comment

Choose a reason for hiding this comment

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

Thanks @Lakshyadevelops! Here are some suggestions to make this code more consistent with pvlib conventions and the other irradiance decomposition functions.

pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
docs/sphinx/source/whatsnew/v0.9.6.rst Outdated Show resolved Hide resolved
@kandersolar kandersolar modified the milestones: 0.9.6, 0.10.0 May 16, 2023
@kandersolar
Copy link
Member

@Lakshyadevelops do you think you'll be able to update this PR for the above comments? Let us know if not and we can finish it. Thanks :)

pvlib/irradiance.py Outdated Show resolved Hide resolved
docs/sphinx/source/reference/irradiance/decomposition.rst Outdated Show resolved Hide resolved
Copy link
Member

@kandersolar kandersolar left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @Lakshyadevelops for this PR!

pvlib/irradiance.py Outdated Show resolved Hide resolved
pvlib/tests/test_irradiance.py Outdated Show resolved Hide resolved
pvlib/irradiance.py Outdated Show resolved Hide resolved
@kandersolar kandersolar changed the title Implement pvl_louche Implement pvlib.irradiance.louche decomposition model May 30, 2023
@kandersolar kandersolar merged commit ad832cd into pvlib:main May 30, 2023
30 checks passed
@Lakshyadevelops Lakshyadevelops deleted the matlab branch June 1, 2023 17:40
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.

None yet

4 participants