-
Notifications
You must be signed in to change notification settings - Fork 47
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
Sunset times returning for previous day #83
Comments
Hi, Writing this message just to confirm you that there is an issue and you're not doing it wrong.
My example is for Australia/Adelaide:
Which is correct Astral == 3.2
We can clearly see that for Dawn and sunrise, the date is in correct (the offset is about one day) |
#81 and #82 are duplicates of this. I started looking into this, but am not familiar enough with the calculates to make quick progress. My hunch is that this refactor of The best answer (or interim workaround) may be to calculate sunrise and sunset times for date-1, date, and date+1, and then calculate the timezone at the location, find the UTC time at midday, and take the closes sunrise and sunset times around that timestamp. These are the unit tests for @pytest.mark.parametrize(
"day,sunrise",
[
(datetime.date(2015, 1, 1), datetime.datetime(2014, 12, 31, 16, 51)),
(datetime.date(2015, 12, 1), datetime.datetime(2015, 11, 30, 16, 42)),
(datetime.date(2015, 12, 2), datetime.datetime(2015, 12, 1, 16, 42)),
(datetime.date(2015, 12, 3), datetime.datetime(2015, 12, 2, 16, 42)),
(datetime.date(2015, 12, 12), datetime.datetime(2015, 12, 11, 16, 41)),
(datetime.date(2015, 12, 25), datetime.datetime(2015, 12, 24, 16, 45)),
],
)
def test_Sunrise_west(day: datetime.date, sunrise: datetime.datetime, wellington: LocationInfo):
sunrise = sunrise.replace(tzinfo=datetime.timezone.utc)
sunrise_utc = sun.sunrise(wellington.observer, day)
assert datetime_almost_equal(sunrise, sunrise_utc)
@pytest.mark.parametrize(
"day,sunset",
[
(datetime.date(2015, 1, 1), datetime.datetime(2015, 1, 1, 7, 56)),
(datetime.date(2015, 12, 1), datetime.datetime(2015, 12, 1, 7, 36)),
(datetime.date(2015, 12, 2), datetime.datetime(2015, 12, 2, 7, 37)),
(datetime.date(2015, 12, 3), datetime.datetime(2015, 12, 3, 7, 38)),
(datetime.date(2015, 12, 12), datetime.datetime(2015, 12, 12, 7, 47)),
(datetime.date(2015, 12, 25), datetime.datetime(2015, 12, 25, 7, 55)),
],
)
def test_Sunset_west(day: datetime.date, sunset: datetime.datetime, wellington: LocationInfo):
sunset = sunset.replace(tzinfo=datetime.timezone.utc)
sunset_utc = sun.sunset(wellington.observer, day)
assert datetime_almost_equal(sunset, sunset_utc) |
Reproducible with
|
@nigelsim want to check your unit tests with this fork? I think i fixed it. I think you found the right refactor. |
Since astral 2.2, computing the sunset time appears to be return the time for the previous day. More accurately, if the sunset time ends up being UTC the next day, the day component of the date isn't changed. For example... Jul 10,2022 for my location sunset returns as Jul 10, 2022 03:10 UTC. I'm in the -7:00 timezone, so that would mean sunset for July 10 was computed as July 9 20;10. Clearly sunset on July 10 couldn't start the day before. I think the day component of the time should have been 11, not 10.
This code replicates the issue.
On astral 3.0, 3.1 and 3,2 it returns
datetime.datetime(2022, 7, 10, 3, 10, 4, 552844, tzinfo=datetime.timezone.utc)
On astral 2.2 it returns
datetime.datetime(2022, 7, 11, 3, 9, 47, 57594, tzinfo=<UTC>)
I believe the return value from astral 2.2 is the correct value.
I'm open to the possibility I'm doing it wrong, but I didn't see anything in release notes that said I needed to change anything. Noticed this when upgrading from Astral 2.2 to 3.2 -- previously working code began to fail.
Thanks for any help / insights!
The text was updated successfully, but these errors were encountered: