Skip to content

improve solarposition.hour_angle and other analytical test functions #597

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.

Sign up for GitHub

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

Jump to bottom
Closed
wholmgren opened this issue Oct 4, 2018 · 0 comments · Fixed by #599
Closed

improve solarposition.hour_angle and other analytical test functions #597

wholmgren opened this issue Oct 4, 2018 · 0 comments · Fixed by #599
Labels
testing
Milestone
0.6.1

Comments

@wholmgren
Copy link
Member

Problems:

solarposition.hour_angle should have its own test in test_solarposition. Its output is tested in test_bird but that's not the right place for it.

solarposition.hour_angle is used in test_analytical_zenith and test_analytical_azimuth. That's bad practice. The success of the zenith and azimuth functions should not depend on the hour_angle function.

This situation is making it hard for me to track down an issue. There might be something wrong with solarposition.hour_angle timezone handling, but it's hard to say at the moment.

Versions:

  • pvlib.__version__: 0.6

Additional context
discovered when addressing pandas upgrade issues in #595
@wholmgren wholmgren added this to the 0.6.1 milestone Oct 4, 2018
@wholmgren wholmgren mentioned this issue Oct 4, 2018
Closed
mikofski added a commit to mikofski/pvlib-python that referenced this issue Oct 5, 2018
@mikofski
add test for hour_angle, vectorize
ace0a17 
* closes pvlib#598 vectorize to make it more efficient
* closes pvlib#597 add test

Signed-off-by: Mark Mikofski <[email protected]>
@mikofski mikofski mentioned this issue Oct 5, 2018
Merged
8 tasks
wholmgren pushed a commit that referenced this issue Oct 5, 2018
@mikofski @wholmgren
add test for hour_angle, vectorize (#599)
5ee82b1 
* add test for hour_angle, vectorize

* closes #598 vectorize to make it more efficient
* closes #597 add test

Signed-off-by: Mark Mikofski <[email protected]>

* BUG: use np.int64 works better for older numpy version than python int

* also converting times to int before subtracting works better for older
pandas versions which were not calculating the timedeltas correctly
* remove comment with missing space after hash, add FIXME that explains
why the expected values are slightly different than the SPA calculator
output

* fix hanging indent

* BUG: replace utcoffset() with reliable, efficient approach ...

* ... suggested by @wholmgren (thx!)
* utcoffset() is unpredictable when used with pandas datetime indices
* only predictable with Python datetime objects or pandas Timestamps
* instead replace tzinfo with None to get naive local times, and
calculate difference from tz-aware times to get timezones

* BUG: combine arithmetic to make calculation more efficient

* also use asarray wrapper at return to ensure consistency
* add comments to explain to future maintainers

* stickler

* remove try-except, doesn't work anyway, wait for pandas >=0.15.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
testing
Projects
None yet
Milestone
0.6.1
Development

Successfully merging a pull request may close this issue.

add test for hour_angle, vectorize mikofski/pvlib-python
1 participant
@wholmgren