-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Behavior of pvlib.solarposition.get_sun_rise_set_transit #316
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
Comments
I'm not very familiar with this function. I think @alorenzo175 contributed it, so maybe he can help you. |
Only the date part of your time stamps is used to identify the day in question at your location. This day spans the period 2017-02-12 07:00:00 UTC to 2017-02-13 07:00:00, and your sunset is within this period. Probably not the ideal behaviour for this function. |
This is less than ideal, but makes sense when you consider the entire This should at least be better documented. The sunrise, sunset, and transit could also be split into separate functions where it would then make sense to always return a result on the input day. |
I can follow the logic now, but the function doesn't make sense, to me. The function returns the sunrise time based on the date alone, so the returned sunrise which can be before or after the specified time. E.g.,
returns for sunrise both The non-intuitive result is that the timestamp 2017-02-12 00:00:00+00:00 is daylight at the location of interest, so there's a sunset at 2017-02-12 01:19(ish)+00:00 which isn't returned. What is the intended use? In my application I have a list of times and I need all sunrises and sunsets which occur between the beginning and end of the list. This function doesn't meet that need. |
The timezone behavior also seems like a potential source of error:
Seems fixable by passing the full datetime input, rather than the so-called pyephem has some functions for this, too. |
FYI: when I call the compiled SPA C-files (as of v0.6.0, I do get the expected behavior: from pvlib.spa_c_files.spa_py import spa_calc
result = spa_calc(
year=2018, month=1, day=1, hour=0, minute=0, second=0,
time_zone=-7, longitude=-105.178, latitude=39.743,
elevation=1830.14, pressure=820, temperature=11, delta_t=67)
print(result['sunset'], result['sunrise']) # SPA hours
#16.785243982117017 7.364247922706669
print(timedelta(hours=16.785242) +
pytz.timezone('Etc/GMT+7').localize(datetime(2018, 1, 1, 0, 0, 0)))
#2018-01-01 16:47:06.871200-07:00
print(timedelta(hours=7.364246) +
pytz.timezone('Etc/GMT+7').localize(datetime(2018, 1, 1, 0, 0, 0)))
#2018-01-01 07:21:51.285600-07:00 |
@mikofski the |
I think I understand the cause for the odd behavior. This line:
produces different dates:
which, if passed through to the sunrise calculation, returns sunrise times on different dates:
|
Does this work?
We'd need to bump the minimum pandas version to use |
I don't think that helps, it's the same behavior - references the date to midnight UTC, rather than the date at the location/time of interest. At 6pm tonight (9/20/2018) where I am, UTC has tomorrow's date, if that makes sense. |
Sorry, I transposed the tz_convert and the floor. I thought it would be different than the currently implemented code because it's not dropping tz awareness when calculating the date. Didn't test though so sorry if I'm just spamming. |
The desired behavior of I'll prepare a pull request to modify how I'm posting this comment to ask: is requiring the |
Ok with me. |
Closed by #603 |
Why does this function return a sunset time >24 hours ahead of the specified time?
E.g.,
returns
The comments in get_sun_rise_set_transit don't describe what the output should be.
The text was updated successfully, but these errors were encountered: