-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Handle overflow condition in pvlib.clearsky.haurwitz #364
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
Conversation
This reverts commit d83d77d.
Handles overflow when apparent zenith is slight greater than 90. Revised test case to operate on zenith rather than time, removes dependence on solarposition.
Resolve errors in test definition
Remove use of np.where in haurwitz, fix declaration of pandas objects
Changed np arrays from 2d to 1d for use as dataframe indices
Resolving type conflict inside haurwitz function for casting output as a dataframe
Ready for review |
Thanks, Cliff. Looks good, I only have a couple of small style nitpicks... The line with the calculation is too long (> 79 characters). No spaces around equal signs inside a function call, so change Please delete the old, commented test code from the test file. Can you create a new I see that there is a preexisting mistake in the function's documentation. The documentation claims that the function will return a Series, however, it always returns a DataFrame (I think this was done to be consistent with other clear sky functions). Can you go ahead and fix this? I see that you first tried to use |
Will do.
np.where() evaluates both outcomes for all arguments before it applies the condition, so the overflow warning would still appear.
|
Fix line length, formatting and documentation in clearsky.haurwitz. Create whatsnew\v0.5.1.rst. Delete commented old code from test\test_clearsky.py
Thanks, Cliff! |
Revised test to operate on zenith rather than time, to remove dependence on solarposition