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

Updating Townsends snow model references #2384

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ayushjariyal
Copy link

pvlib/snow.py Outdated Show resolved Hide resolved
pvlib/snow.py Outdated Show resolved Hide resolved
@ayushjariyal ayushjariyal requested a review from cwhanse February 9, 2025 17:51
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.

@ayushjariyal you will need to fix the line length and indentation in reference [2]

Please add a note to the whatsnew file for v0.11.3

pvlib/snow.py Outdated
Comment on lines 290 to 291
.. [2] Townsend, T. (2025). Snow Loss Model Enhancements.
Available at: https://www.nrel.gov/docs/fy25osti/90585.pdf
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. [2] Townsend, T. (2025). Snow Loss Model Enhancements.
Available at: https://www.nrel.gov/docs/fy25osti/90585.pdf
.. [2] Townsend, T. and Previtali, J. (2023). A Fresh Dusting: Current Uses of the Townsend
Snow Model. In "Photovoltaic Reliability Workshop (PVRW) 2023 Proceedings: Posters.",
ed. Silverman, T. J. Dec. 2023. NREL/CP-5900-87918.
Available at: https://www.nrel.gov/docs/fy25osti/90585.pdf

Copy link
Member

Choose a reason for hiding this comment

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

You will need to adjust the line lengths in reference [2] to 79 characters or less.

@ayushjariyal ayushjariyal requested a review from cwhanse February 10, 2025 03:12
Comment on lines 20 to 21
* Fix fornatting in the References section to ensure proper line length and indentation,
specifically for reference [2] in the documentation.(:issue:`2383`, :pull:`2384`)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Fix fornatting in the References section to ensure proper line length and indentation,
specifically for reference [2] in the documentation.(:issue:`2383`, :pull:`2384`)
* Add references for :py:func:`pvlib.snow.loss_townsend`. (:issue:`2383`, :pull:`2384`)

Copy link
Member

Choose a reason for hiding this comment

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

If you agree we'd like you to add your GH and name to the Contributor's section.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you, @cwhanse ! I’d be honored to add my name and GH to the Contributors' section. I really appreciate the opportunity to contribute to this project!

@ayushjariyal ayushjariyal requested a review from cwhanse February 10, 2025 09:18
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.

You can add a line after this one in the whatsnew file

* Manoj K S (:ghuser:`manojks1999`)

pvlib/snow.py Outdated
Comment on lines 290 to 292
.. [2] Townsend, T. and Previtali, J. (2023). A Fresh Dusting: Current Uses of the Townsend
Snow Model. In "Photovoltaic Reliability Workshop (PVRW) 2023 Proceedings: Posters.",
ed. Silverman, T. J. Dec. 2023. NREL/CP-5900-87918.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. [2] Townsend, T. and Previtali, J. (2023). A Fresh Dusting: Current Uses of the Townsend
Snow Model. In "Photovoltaic Reliability Workshop (PVRW) 2023 Proceedings: Posters.",
ed. Silverman, T. J. Dec. 2023. NREL/CP-5900-87918.
.. [2] Townsend, T. and Previtali, J. (2023). A Fresh Dusting: Current Uses of the
Townsend Snow Model. In "Photovoltaic Reliability Workshop (PVRW) 2023
Proceedings: Posters.", ed. Silverman, T. J. Dec. 2023. NREL/CP-5900-87918.

@ayushjariyal ayushjariyal requested a review from cwhanse February 10, 2025 17:18
@cwhanse
Copy link
Member

cwhanse commented Feb 10, 2025

@ayushjariyal can you fix the line length issues in your editor? I'm having difficulty suggesting corrections here.

@ayushjariyal
Copy link
Author

@cwhanse I’m having trouble understanding why the tests are failing.

Screenshot from 2025-02-10 23-16-15

Could you please help clarify the issue?

@cwhanse cwhanse added this to the v0.11.3 milestone Feb 10, 2025
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.

Thanks @ayushjariyal

@cwhanse
Copy link
Member

cwhanse commented Feb 10, 2025

@cwhanse I’m having trouble understanding why the tests are failing.
Could you please help clarify the issue?

You can ignore those failures. They are unrelated to the changes you made and they are happening in other PRs.

@ayushjariyal
Copy link
Author

Thank you for the clarification!

Copy link
Contributor

@IoannisSifnaios IoannisSifnaios left a comment

Choose a reason for hiding this comment

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

LGTM! Only one suggestion from my side.

pvlib/snow.py Outdated
Comment on lines 295 to 296
.. [3] Townsend, T. (2025). Snow Events Definition.
:doi:`10.13140/RG.2.2.14299.68647`
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we use the actual title of the poster and also mention that it was presented in SPI conference in 2013 (see info in the image below)

image

Copy link
Member

Choose a reason for hiding this comment

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

I agree - I overlooked the "SPI 2013" conference label. Reference text should be

[3] Townsend, T. (2013). Predicting PV Energy Loss Caused by Snow. Solar Power International, Chicago IL. :doi:10.13140/RG.2.2.14299.68647

@ayushjariyal ayushjariyal requested a review from cwhanse February 12, 2025 05:19
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.

loss_townsend: Update Townsend Snow Model References
3 participants