-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Allow scalar surface orientation inputs to pvlib.bifacial.pvfactors_timeseries #1361
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
Allow scalar surface orientation inputs to pvlib.bifacial.pvfactors_timeseries #1361
Conversation
pvlib/bifacial.py
Outdated
| dni = np.array(dni) | ||
| dhi = np.array(dhi) | ||
| # GH 1127, GH 1332 | ||
| if np.isscalar(surface_tilt): |
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.
Use numpy.atleast_1d? It would avoid the if but otherwise I don't know if it's better.
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 think the requirement is that surface_tilt be an array of the same length as the meteo inputs, so np.atleast_1d would still need to be followed by a conditional np.repeat or something.
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.
full_like docs say it requires a scalar input, but this works:
a = np.array([1,2,3])
np.full_like(a, 7)
array([7, 7, 7])
np.full_like(a, np.array([7,8,9]))
array([7, 8, 9])
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.
Seems to work with lists and Series too. Always happy to learn a new numpy function :)
pvlib/bifacial.py
Outdated
| dni = np.array(dni) | ||
| dhi = np.array(dhi) | ||
| # GH 1127, GH 1332 | ||
| if np.isscalar(surface_tilt): |
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.
full_like docs say it requires a scalar input, but this works:
a = np.array([1,2,3])
np.full_like(a, 7)
array([7, 7, 7])
np.full_like(a, np.array([7,8,9]))
array([7, 8, 9])
|
New test failures are the same as #717 (comment), so ignoring here |
Updates entries todocs/sphinx/source/api.rstfor 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`).New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.As mentioned in #1332, I opted to rework the function to allow scalar orientation inputs instead of editing the docstring to prohibit them. I also did some cleanup in the tests to avoid code duplication.