Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout `pvlib.irradiance` #2191

RDaxini · 2024-08-29T19:02:57Z

Partially closes Missing Descriptions for Formula Components in haydavies #2183, related to Klucher diffuse sky docstring inconsistency #1405
I am familiar with the contributing guidelines
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.

add eqn variable definitions rearrange references changed beta into theta_T to avoid double definition --- surface tilt, and "daily average of the solar elevation in degrees" (pvlib.irradiance.diffuse_par_spitters)

Remove Loutzenhiser P.G. et. al. reference. I do not see it cited anywhere here in the docs for the haydavies model

add "projection ratio" to variable definition

separate A and R_b

RDaxini · 2024-08-29T19:27:46Z

Is anyone aware of an active (permanent?) link to the original Hay and Davies publication? I think it would help to link that into the references but I have been unable to find a suitable link yet.

Change all instances of W/m^2 into Wm⁻²

RDaxini · 2024-08-29T19:31:17Z

f89f3f7, 87fc372 is my personal stylistic preference for typesetting the units of watt per metre squared. This can be reverted if people disagree.

W/m^2 to Wm⁻²

move eqn and add description and variables definition to a new notes section update references redefine surface tilt for consistency with pvlib#2191

kandersolar · 2024-08-30T11:56:04Z

Is anyone aware of an active (permanent?) link to the original Hay and Davies publication? I think it would help to link that into the references but I have been unable to find a suitable link yet.

It's available at archive.org, although you have to create an account and "check it out": https://archive.org/details/proceedingsfirst00cana/mode/2up

I do not know if a more convenient alternative is available.

RDaxini · 2024-09-02T11:10:58Z

It's available at archive.org, although you have to create an account and "check it out": https://archive.org/details/proceedingsfirst00cana/mode/2up

I do not know if a more convenient alternative is available.

I have added that one in for now (1063804)
I'll add an up to date "last accessed" just before this PR is merged

cwhanse

Concur with the additional description of terms in the Hay/Davies model equation. I disagree with changing the primary reference.

pvlib/irradiance.py

kandersolar

I'm neutral on the units formatting changes. The new form is a little nicer to look at, but I foresee myself grumbling for having to copy/paste that superscript 2 in future code edits :P

pvlib/irradiance.py

Co-authored-by: Kevin Anderson <[email protected]>

pvlib/irradiance.py

adjust units Co-Authored-By: Echedey Luis <[email protected]>

RDaxini · 2024-09-15T12:09:09Z

@echedey-ls I removed the :math: but then added [] rather than () as I think that is more common across other pvlib functions.

RDaxini · 2024-09-15T12:15:16Z

What's up with the pytest command not found error since 39a7ddb? :S

echedey-ls · 2024-09-15T14:24:59Z

@echedey-ls I removed the :math: but then added [] rather than () as I think that is more common across other pvlib functions.

Yup, I prefer that too. poa_components still has parenthesis thou, that's why I thought the next code belonged to it.

What's up with the pytest command not found error since 39a7ddb? :S

Once again, witchcraft. I will blind shot that in a PR.

RDaxini · 2024-09-15T15:17:14Z

Yup, I prefer that too. poa_components still has parenthesis thou, that's why I thought the next code belonged to it.

I think the original author might have done that because in that case it looks like the units is part of the sentence rather than just at the end of a parameter name with/without additional info after it that can be read and makes sense in isolation.

I'm sure we'll be able to do a thorough review of units/sentences like this following #2209. I will neaten up that PR and mark it ready for review soon.

pvlib/irradiance.py

Co-authored-by: Kevin Anderson <[email protected]>

docs/sphinx/source/whatsnew/v0.11.1.rst

cwhanse

If you care to, I would change the description of return_components to

If `True`, ``diffuse_components`` is returned.

The markup in the first Returns section could be improved using double `` rather than single `, in places.

pvlib/irradiance.py

Co-Authored-By: Cliff Hansen <[email protected]>

RDaxini · 2024-09-24T16:08:22Z

If you care to, I would change the description of return_components to
If `True`, ``diffuse_components`` is returned.

Done

The markup in the first Returns section could be improved using double `` rather than single `, in places.

I think I have resolved this now with ` for parameter values and `` for parameters. Let me know if I missed anything. If not, I think all issues have now been resolved.

cwhanse · 2024-09-24T16:14:20Z

Changes to the Returns section are great. I was also suggesting that we edit the description of the input parameter return_components.

RDaxini · 2024-09-24T16:27:41Z

Changes to the Returns section are great. I was also suggesting that we edit the description of the input parameter return_components.

ohh right of course, I just misread/misunderstood. Check the last commit --- let me know if we're on the same page now 😅

kandersolar · 2024-09-24T18:01:01Z

Thanks @RDaxini!

RDaxini · 2024-09-24T18:27:06Z

Thanks @RDaxini!

and thanks for all the reviews!
btw good idea with the rename 👍🏾

* Update irradiance.py move eqn and add description and variables definition to a new notes section update references redefine surface tilt for consistency with #2191 * Update irradiance.py update variable names (I_d0 to DHI and add definition of theta as aoi) * Update irradiance.py add doi * Update irradiance.py wording in definition of F' * Update irradiance.py * Update irradiance.py \theta->\beta reinstate Loutzenhiser reference * Update irradiance.py add doi to * Update irradiance.py * Update irradiance.py add units to returns statement * typo * Update v0.11.1.rst * Update v0.11.1.rst * update model description * whatsnew, update and merge entries * whatsnew (missing word) * move references (klucher and haydavies) moved references from first line to notes section

RDaxini added 5 commits August 29, 2024 19:59

Update irradiance.py

e5860fc

add eqn variable definitions rearrange references changed beta into theta_T to avoid double definition --- surface tilt, and "daily average of the solar elevation in degrees" (pvlib.irradiance.diffuse_par_spitters)

Update irradiance.py

03d220d

Update irradiance.py

c64e3da

Remove Loutzenhiser P.G. et. al. reference. I do not see it cited anywhere here in the docs for the haydavies model

Update irradiance.py

97f94e8

add "projection ratio" to variable definition

Update irradiance.py

3baddf4

separate A and R_b

Update irradiance.py

87fc372

Change all instances of W/m^2 into Wm⁻²

RDaxini added 2 commits August 29, 2024 20:48

Update irradiance.py

f89f3f7

W/m^2 to Wm⁻²

Update irradiance.py

4b0c32d

RDaxini marked this pull request as ready for review August 30, 2024 11:06

kandersolar added this to the v0.11.1 milestone Aug 30, 2024

RDaxini changed the title ~~[WIP] Update pvlib.irradiance.haydavies docs~~ Update pvlib.irradiance.haydavies docs Aug 30, 2024

kandersolar added the documentation label Aug 30, 2024

RDaxini added a commit to RDaxini/pvlib-python that referenced this pull request Aug 30, 2024

Update irradiance.py

4a4ec58

move eqn and add description and variables definition to a new notes section update references redefine surface tilt for consistency with pvlib#2191

add url to references

1063804

RDaxini changed the title ~~Update pvlib.irradiance.haydavies docs~~ Update irradiance.haydavies docs Aug 30, 2024

Update irradiance.py

ca13315

RDaxini mentioned this pull request Aug 30, 2024

update irradiance.reindl docs #2193

Merged

6 tasks

unit typesetting suggestion

18ee5e8

RDaxini mentioned this pull request Aug 30, 2024

Add Muneer transposition model #2184

Draft

8 tasks

RDaxini mentioned this pull request Sep 3, 2024

Update irradiance.klucher docs #2192

Merged

6 tasks

cwhanse reviewed Sep 3, 2024

View reviewed changes

pvlib/irradiance.py Outdated Show resolved Hide resolved

RDaxini added 2 commits September 3, 2024 15:53

Update irradiance.py

3a9b965

Update irradiance.py

3037922

kandersolar reviewed Sep 3, 2024

View reviewed changes

pvlib/irradiance.py Outdated Show resolved Hide resolved

pvlib/irradiance.py Outdated Show resolved Hide resolved

RDaxini and others added 2 commits September 3, 2024 21:00

Update pvlib/irradiance.py

5012ee8

Co-authored-by: Kevin Anderson <[email protected]>

eqn typesetting

a32cdf1

echedey-ls reviewed Sep 12, 2024

View reviewed changes

pvlib/irradiance.py Outdated Show resolved Hide resolved

Update irradiance.py

39a7ddb

adjust units Co-Authored-By: Echedey Luis <[email protected]>

Update irradiance.py

8df6115

echedey-ls mentioned this pull request Sep 15, 2024

Failing pytest CI on mac-os #2211

Closed

kandersolar reviewed Sep 16, 2024

View reviewed changes

pvlib/irradiance.py Outdated Show resolved Hide resolved

pvlib/irradiance.py Show resolved Hide resolved

RDaxini added 3 commits September 17, 2024 11:36

\theta_T -> \beta

faab882

add last accessed to reference

ef54fe9

remove spurious period

095f348

kandersolar approved these changes Sep 17, 2024

View reviewed changes

pvlib/irradiance.py Outdated Show resolved Hide resolved

pvlib/irradiance.py Outdated Show resolved Hide resolved

Apply suggestions from code review

67d5b4b

Co-authored-by: Kevin Anderson <[email protected]>

RDaxini commented Sep 23, 2024

View reviewed changes

docs/sphinx/source/whatsnew/v0.11.1.rst Show resolved Hide resolved

kandersolar mentioned this pull request Sep 23, 2024

v0.11.1 release plan #2220

Closed

11 tasks

Merge remote-tracking branch 'upstream/main' into update_haydavies_docs

a927efc

cwhanse approved these changes Sep 24, 2024

View reviewed changes

pvlib/irradiance.py Outdated Show resolved Hide resolved

pvlib/irradiance.py Outdated Show resolved Hide resolved

Update irradiance.py

d142df9

Co-Authored-By: Cliff Hansen <[email protected]>

Update irradiance.py

4eb06af

cwhanse approved these changes Sep 24, 2024

View reviewed changes

kandersolar changed the title ~~Update irradiance.haydavies docs~~ Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout pvlib.irradiance Sep 24, 2024

kandersolar merged commit ad89390 into pvlib:main Sep 24, 2024
29 of 30 checks passed

RDaxini deleted the update_haydavies_docs branch September 24, 2024 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout `pvlib.irradiance` #2191

Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout `pvlib.irradiance` #2191

RDaxini commented Aug 29, 2024 •

edited

Loading

RDaxini commented Aug 29, 2024

RDaxini commented Aug 29, 2024 •

edited

Loading

kandersolar commented Aug 30, 2024

RDaxini commented Sep 2, 2024

cwhanse left a comment

kandersolar left a comment

RDaxini commented Sep 15, 2024

RDaxini commented Sep 15, 2024

echedey-ls commented Sep 15, 2024

RDaxini commented Sep 15, 2024

cwhanse left a comment •

edited

Loading

RDaxini commented Sep 24, 2024 •

edited

Loading

cwhanse commented Sep 24, 2024

RDaxini commented Sep 24, 2024 •

edited

Loading

kandersolar commented Sep 24, 2024

RDaxini commented Sep 24, 2024

Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout pvlib.irradiance #2191

Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout pvlib.irradiance #2191

Conversation

RDaxini commented Aug 29, 2024 • edited Loading

RDaxini commented Aug 29, 2024

RDaxini commented Aug 29, 2024 • edited Loading

kandersolar commented Aug 30, 2024

RDaxini commented Sep 2, 2024

cwhanse left a comment

Choose a reason for hiding this comment

kandersolar left a comment

Choose a reason for hiding this comment

RDaxini commented Sep 15, 2024

RDaxini commented Sep 15, 2024

echedey-ls commented Sep 15, 2024

RDaxini commented Sep 15, 2024

cwhanse left a comment • edited Loading

Choose a reason for hiding this comment

RDaxini commented Sep 24, 2024 • edited Loading

cwhanse commented Sep 24, 2024

RDaxini commented Sep 24, 2024 • edited Loading

kandersolar commented Sep 24, 2024

RDaxini commented Sep 24, 2024

Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout `pvlib.irradiance` #2191

Update irradiance.haydavies docs; use Wm⁻² instead of W/m^2 throughout `pvlib.irradiance` #2191

RDaxini commented Aug 29, 2024 •

edited

Loading

RDaxini commented Aug 29, 2024 •

edited

Loading

cwhanse left a comment •

edited

Loading

RDaxini commented Sep 24, 2024 •

edited

Loading

RDaxini commented Sep 24, 2024 •

edited

Loading