-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
ayushjariyal
commented
Feb 9, 2025
- Closes loss_townsend: Update Townsend Snow Model References #2383
There was a problem hiding this 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
There was a problem hiding this 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`)
@ayushjariyal can you fix the line length issues in your editor? I'm having difficulty suggesting corrections here. |
af253e3
to
7d699c8
Compare
@cwhanse I’m having trouble understanding why the tests are failing. Could you please help clarify the issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ayushjariyal
You can ignore those failures. They are unrelated to the changes you made and they are happening in other PRs. |
Thank you for the clarification! |
There was a problem hiding this 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
.. [3] Townsend, T. (2025). Snow Events Definition. | ||
:doi:`10.13140/RG.2.2.14299.68647` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
pvlib/snow.py
Outdated
Available at: https://www.nrel.gov/docs/fy25osti/90585.pdf | ||
.. [3] Townsend, T. (2013). Predicting PV Energy Loss Caused by Snow. | ||
Solar Power International, Chicago IL. | ||
doi:`10.13140/RG.2.2.14299.68647` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doi:`10.13140/RG.2.2.14299.68647` | |
:doi:`10.13140/RG.2.2.14299.68647` |
One last edit I think.
Thanks @ayushjariyal ! |
@cwhanse Thank you for reviewing and merging my PR! I really appreciate the support. Also, does this organization participate in GSoC? I'm excited to contribute further! |
@ayushjariyal we do participate in GSoC, the current list of project ideas is here. |
* Updating Townsends snow model references * Applying reviews * Adding a note to the whatsnew file for v0.11.3 * Applying review * Add myself as contributor and apply reviews * Fix the line length issue * Adding actual title of the poster * correct doi syntax