Add PVSPEC spectral correction factor model#2072
Conversation
first attempt, PVSPEC model for the spectral mismatch factor based on air mass and clearness index
|
There are a few things that need to be done first:
In general, you may want to see some PRs that add new functions as a reference. For example, #2041 is simple IMO. There are many more of course, it's just I'm too selfish to look for other ones (/sarcasm) I've been inspired by #2067 to do a post blog regarding my setup. I know I wrote too much, but in case it helps you, check it out here. |
add new spectral factor (to be tested)
delete new py file (and add new model into mismatch.py)
correct errors raised
create new tests for pelland air mass / clearness index spectral correction
Correct an expected value
|
@echedey-ls @cwhanse Thank you for the help. I think I have updated the files with the correct names now (and also put the functions in the right places) |
|
Well done @RDaxini , it looks good!!
Yeah, documented functions aren't discovered automatically, you need to specify in a rST file the function. In general, these files are under I will review in short some of the common typos we all make in the docstrings. |
echedey-ls
left a comment
There was a problem hiding this comment.
Some general feedback. I don't want to overwhelm you, but I've left a fair amount of comments.
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: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
update eqn in docs
tried typesetting the url differently, will fix line character length after
|
Don't worry @RDaxini , we've all been there. Btw, can you double check the "sodapro" link is what you intended? http://www.sodapro.com/gl/web-services/atmosphere/linke-turbidity-factor-ozone-watervapor-and-angstroembeta It doesn't work (404). Ah, and Sodapro looks like a sandblasting company? |
|
I believe https://www.soda-pro.com/ is the correct website ;) |
cwhanse
left a comment
There was a problem hiding this comment.
Editorial mostly, I think that matching output type to input type has slso been raised by others.
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
specific data resource via extended url is either behind a paywall or moved; linking the main site homepage instead
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
|
@kandersolar @AdamRJensen or @cwhanse can you approve the workflows on this PR? |
|
@RDaxini one last thing is needed: a "what's new" entry for v0.11.0. Can you make a new bullet point under |
kandersolar
left a comment
There was a problem hiding this comment.
LGTM with one minor syntax edit. Thanks @RDaxini!
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
|
Thanks for all the reviews @echedey-ls @cwhanse @IoannisSifnaios @kandersolar |
remote-data) and Milestone are assigned to the Pull Request and linked Issue.This model is one of the spectral correction functions suggested in the cited issues (#1950 and #2065).
Docs: https://pvlib-python--2072.org.readthedocs.build/en/2072/reference/generated/pvlib.spectrum.spectral_factor_pvspec.html