Skip to content
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

Timezone ignored in intervals() #28

Closed
LTMullineux opened this issue Sep 9, 2022 · 3 comments
Closed

Timezone ignored in intervals() #28

LTMullineux opened this issue Sep 9, 2022 · 3 comments

Comments

@LTMullineux
Copy link

When passing timezone-aware start and/or end to the .intervals() function the returned datetime objects are not timezone aware:

>> from opening_hours import OpeningHours
>> from datetime import datetime, timedelta
>> import pytz
>> oh = OpeningHours('Mo-Fr 09:00-17:00')
>> start = datetime(2022, 9, 5, 0, 0, 0, 0, pytz.UTC)
>> end = start + timedelta(days=2)
>> next(oh.intervals(start=start, end=end))
(datetime.datetime(2022, 9, 5, 0, 0), datetime.datetime(2022, 9, 5, 9, 0), 'closed', [])

So in the above if you were to do something like the below you would get a Python TypeError when doing inequalities, or a False when comparing equality:

>> rust_start, rust_end, state, _ = next(oh.intervals(start=start, end=end))
>> rust_start == start
False
>> rust_start >= start
TypeError: can't compare offset-naive and offset-aware datetimes

My Rust isn't good enough to help with the debugging in the code, but it looks like there is not a way to add the timezone cleanly to NaiveDateTime?

If not it will be possible to do the timezone adjustments on the returned dates in Python, but it would be cool if this was handled straight out of the box.

@remi-dupre
Copy link
Owner

Hi! Thanks for your feedback.

I don't think it would be a good idea to add a timezone by default: it will be error prone because most of the time these opening hours fields come from OSM or equivalent and depend on the locality of the point of interest, not of the locality of the user.

However I agree that something opt-in may be relevant, mostly because timezone management is always a pain and anything that can make it easier will probably make it welcome (maybe just allow to pass a timezone to the parser so that it outputs times in given timezone).

@LTMullineux
Copy link
Author

Hi, thanks for getting back.

I agree with your idea of having the option to pass in a timezone and have whatever datetime objects that are yielded be in that timezone

@remi-dupre
Copy link
Owner

Hello!

I've just added this while relying on PyO3's conversion layer for timezones, which currently has a few limitations :

Anyway, your example, very slightly modified now works :

>> from opening_hours import OpeningHours
>> from datetime import datetime, timedelta
>> from zoneinfo import ZoneInfo
>> oh = OpeningHours('Mo-Fr 09:00-17:00')
>> start = datetime(2022, 9, 5, 0, 0, 0, 0, ZoneInfo("UTC"))
>> end = start + timedelta(days=2)
>> next(oh.intervals(start, end))
(datetime.datetime(2022, 9, 5, 0, 0, tzinfo=datetime.timezone.utc), datetime.datetime(2022, 9, 5, 9, 0, tzinfo=datetime.timezone.utc), State.CLOSED, [])

I've played a bit around it and this comes with a few more features :

  • You can now specify a local timezone with the expression, any input time will be converted to that timezone (eg. for keeping the user's time consistent with the actual place's location)
  • If you specify a local timezone, sun events will be precisely computed (sunset, ...)
  • Timezone can be inferred from coordinates automatically, together with a public holidays database.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants