Simplify/DRY some infinite_sheds/bifacial.utils calculations#2559
Simplify/DRY some infinite_sheds/bifacial.utils calculations#2559kandersolar wants to merge 5 commits intopvlib:mainfrom
Conversation
|
You've probably got the email, but docs are failing with: This failing example
Extension error:
Here is a summary of the problems encountered when running the examples:
Unexpected failing examples (1):
../../examples/agrivoltaics/plot_agrivoltaics_ground_irradiance.py failed leaving traceback:
Traceback (most recent call last):
File "/home/docs/checkouts/readthedocs.org/user_builds/pvlib-python/checkouts/2559/docs/examples/agrivoltaics/plot_agrivoltaics_ground_irradiance.py", line 145, in <module>
unshaded_ground_fraction = pvlib.bifacial.utils._unshaded_ground_fraction(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: _unshaded_ground_fraction() got an unexpected keyword argument 'surface_azimuth'My opinion is that it's bad practice to use internals in public-facing examples. But a fix, using internals or not, is okay with me. Also, I think this deserves a whatsnew 👀 |
Agreed. We should consider making that ground shaded fraction function public. |
|
Closing as it's nicer to use |
|
You’re gonna hate this, but you could memoize |
Easily hatable TBF 😆 The easiest option relies on python's I'm onboard with Kevin on this approach hehe. Thou that was a good concern @mikofski :) |
[ ] Closes #xxxx[ ] Updates entries indocs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor 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`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.Projected solar zenith angle (
tan_phi) is calculated several times in theinfinite_shedsworkflow. This PR calculates it once and passes it around, simplifying the relevant utility functions and their signatures.For context, this is a step towards future enhancements I intend to make to
pvlib.bifacial, which would benefit from working withtan_phiinstead of the base solar position and module angles.