pysolar icon indicating copy to clipboard operation
pysolar copied to clipboard

util.py/get_sunrise_sunset_transit() uses the wrong day?

Open rm-minus-r-star opened this issue 3 years ago • 6 comments

I ran into a problem using get_sunrise_sunset_transit() in util.py. "same_day" goes to the beginning of the day for the input time, not for the day that's relevant at the specified longitude. This might be desired behaviour with respect to other functions within the package that depend on this. But for my purposes, I work in UTC to avoid ambiguity over DST, but determine sunrise/sunset times and solar angles for a timezone that's +10 from UTC. If I use a UTC time of, say, 2021-12-31 23:00:00+00:00, Sunrise should be more like 2021-12-31 19:03:22+00:00, not 2021-12-30 19:03:22+00:00 2021, 12, 30, 19, 3, 22+00:00

For now I've replaced lines 159-162 with the following:

    time_diff = int(round(longitude_deg / 15., 0) - utc_offset / 3600)
    local_day = when + timedelta(hours = time_diff)
    local_day = datetime(year = local_day.year, month = local_day.month, day = local_day.day, tzinfo = local_day.tzinfo)
    print("local_day", local_day)

    transit_time = local_day + timedelta(hours = time_diff + TON)
    transit_time = (transit_time - timedelta(hours = time_diff)).replace(tzinfo=when.tzinfo)
    sunrise_time = transit_time - timedelta(hours = ha)
    sunset_time = transit_time + timedelta(hours = ha)

I think this is right, but I'm not sure of the physics ... I'm not sure whether the earlier day = when.timetuple().tm_yday # Day of the year needs to be similarly adjusted according to the longitude or not.

rm-minus-r-star avatar Apr 21 '21 08:04 rm-minus-r-star

Yeah, it seems like you've identified a problem. get_sunrise_sunset_transit() should check the local time to determine the day, and then determine the sunrise, sunset, and transit times from that. I would welcome a patch that did that.

pingswept avatar Apr 22 '21 19:04 pingswept

We have recently updated out pysolar version from 0.9 to 0.10 and suddenly few tests are broken. I'm struggling to understand if this is the correct behaviour but when doing this:

import pytz
from datetime import datetime
import pysolar.util

day = datetime(2018, 6, 21, 0, 0, tzinfo=pytz.timezone('Europe/Zurich'))
lat, lon = 47.5642388562567, 7.636568666258004
pysolar.util.get_sunrise_sunset(latitude_deg=lat, longitude_deg=lon, when=day)

>> (datetime.datetime(2018, 6, 20, 4, 39, 28, 511700, tzinfo=<DstTzInfo 'Europe/Zurich' LMT+0:34:00 STD>), datetime.datetime(2018, 6, 20, 20, 38, 8, 861432, tzinfo=<DstTzInfo 'Europe/Zurich' LMT+0:34:00 STD>))

As you can see I get the previous day, not the 21. Tested also setting the hour at 12, but then results are shifted:

pysolar.util.get_sunrise_sunset(latitude_deg=lat, longitude_deg=lon, when=day.replace(hour=12))
>> (datetime.datetime(2018, 6, 21, 4, 39, 38, 510017, tzinfo=<DstTzInfo 'Europe/Zurich' LMT+0:34:00 STD>), datetime.datetime(2018, 6, 21, 20, 38, 24, 952483, tzinfo=<DstTzInfo 'Europe/Zurich' LMT+0:34:00 STD>))

In order to make it work (as in 0.9) I have to:

  1. Make sure the datetime object is in the middle of the day.
  2. Convert to UTC before passing it to the get_sunrise_sunset method
  3. Convert results to local timezone

Am I doing something wrong here?

Luigolas avatar Dec 30 '21 12:12 Luigolas

I have the same issue, any news on that?

bhemmer avatar Feb 07 '22 21:02 bhemmer

Fixed this error if you pass "when" in UTC (somehow it's still breaking for utcoffset !=0, and I don't have time to fix that too). It was calculating sunrise/sunset using transit_time in UTC, not in local time. Because ha > transit_time.hour, often if transit_time is in UTC, and you didn't call the function "in the middle of the day", that was causing the sunrise time to end up on the previous day. This can be fixed by making sure you calculate sunrise/sunset time from transit_time in local time, then converting those back to the time zone they were passed in. This gives me back good sunrise/sunset times in UTC that make sense if I pass "when" in UTC.

Lines 167-181

# Get the local day. 
local_day = datetime(year = local_time.year, month = local_time.month, day = local_time.day, tzinfo = local_time.tzinfo)

# Define transit time in Local time (tzinfo says UTC even tho its in local time
# because tzinfo for local_time is never defined in this function, so its wrong, but doesn't matter here. 
# We know transit time is in local time because it is, and should be around 12. 
transit_time = local_day + timedelta(hours = time_diff + TON) 

# Now get sunrise & sunset in local time! 
sunrise_time_local = transit_time - timedelta(hours = ha)    
sunset_time_local = transit_time + timedelta(hours = ha)

# Finally, convert it back to the native time zone it was passed in! 
# Note: time_diff is always positive, while utc_offset can actually be negative- that's why the signs are diff! 
sunrise_time=sunrise_time_local +timedelta(hours=time_diff)-timedelta(seconds=utc_offset)
sunset_time=sunset_time_local +timedelta(hours=time_diff)-timedelta(seconds=utc_offset)

jhaskinsPhD avatar Nov 11 '22 02:11 jhaskinsPhD

i dont know, iff i found the same error. But

from datetime import datetime
from zoneinfo import ZoneInfo
zeit = datetime(2023,5,1,tzinfo=ZoneInfo('Europe/Berlin'))
util.get_transit_time(51,7,zeit).time()

leads to datetime.time(15, 29, 10, 541377) which has a shift of 2 hours.

zeit = datetime(2023,5,1,tzinfo=ZoneInfo('utc'))
util.get_transit_time(51,7,zeit).astimezone(ZoneInfo('Europe/Berlin')).time()

leads to the correct value datetime.time(13, 29, 2, 938833)

I had a look at the sources but i dont really understand what is calculated there. So i came up with my own function get_transit(longitude,d) which is defined as

from datetime import datetime, timedelta, time
from zoneinfo import ZoneInfo
from pysolar import solar

def get_transit(longitude,d):
    ''' return the time of transit of the sun 
        for a given longitude and a given datetime d
        If d is not timezoneaware, the local
        timezone is used
    '''
    
    tz = d.tzinfo if d.tzinfo is not None else d.astimezone().tzinfo
    eot = solar.equation_of_time(d.utctimetuple().tm_yday)
    ret_val = d.replace(hour=12,tzinfo=ZoneInfo('utc'))
    ret_val -= timedelta(minutes=eot) # correction wrt equation of time
    ret_val -= timedelta(hours=longitude/15) # correction wrt longitude
    
    return ret_val.astimezone(tz)

I did not test this code for every timezone and for north / south hemisphere. But maybe it helps to analyse the problem with util.get_transit_time().

The result has the same degree of accuracy as util.get_transit_time().

zeit = datetime(2023,5,1,tzinfo=ZoneInfo('Europe/Berlin'))
tt = get_transit(7,zeit).time()
tt

which leads to datetime.time(13, 29, 11, 993236)

wolfgang-302 avatar Apr 16 '23 11:04 wolfgang-302

Hi, you can change

    sunrise_time = transit_time - timedelta(hours = ha)
    sunset_time = transit_time + timedelta(hours = ha)

to

    sunrise_time = (transit_time - timedelta(hours = ha)) - timedelta(seconds = utc_offset)
    sunset_time = (transit_time + timedelta(hours = ha)) - timedelta(seconds = utc_offset)

That fixed the problem for me.

SelmaUrban avatar Jul 10 '23 06:07 SelmaUrban