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

views: _api_occurrences: Support handling of timezone-aware datetime … #553

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

daym
Copy link

@daym daym commented Mar 8, 2023

…strings (2022-06-07T02:03:04+05:00).

Previously, such strings would fail with ValueError unconverted data remains: +01:00.

This patch changes it so it uses dateutil.parser.parse. The latter can parse all of the following and return a non-surprising result:

>>> dateutil.parser.parse('2020-01-01')
datetime.datetime(2020, 1, 1, 0, 0)
>>> dateutil.parser.parse('2020-01-01 05:42')
datetime.datetime(2020, 1, 1, 5, 42)
>>> dateutil.parser.parse('2020-01-01T05:42')
datetime.datetime(2020, 1, 1, 5, 42)
>>> dateutil.parser.parse('2020-01-01T05:42:00')
datetime.datetime(2020, 1, 1, 5, 42)
>>> dateutil.parser.parse('2020-01-01T05:42:00+01:00')
datetime.datetime(2020, 1, 1, 5, 42, tzinfo=tzoffset(None, 3600))

Additionally, since now it's possible for the convert function to return a datetime with tzinfo that is set, make sure not to try to add another (conflicting) tzinfo in that case.

@dwasyl
Copy link

dwasyl commented May 11, 2023

@daym Have you been using this with recurring events at all? I had applied a similar change, and whenever Event.get_occurrences runs, it fails when generating new occurrences because the tz generated by .parse() is tzoffset(None, 3600) (or whichever TZ it's in) rather than a specific timezone.

o_start = pytz.timezone(str(tzinfo)).localize(o_start) 

Generates an UnknownTimeZone exception as tzinfo is tzoffset(None, 3600) rather than an actual timezone.

Just for anyone in the future, I ended up dealing with it by using the offset to calculate UTC and then replacing the tzinfo with UTC.

So the code in the PR looks like this in my environment:

d = dateutil.parser.parse(ddatetime)
d = d.replace(tzinfo=pytz.UTC) - d.utcoffset()
return d

@daym
Copy link
Author

daym commented May 12, 2023

@daym Have you been using this with recurring events at all? I had applied a similar change, and whenever Event.get_occurrences runs, it fails when generating new occurrences because the tz generated by .parse() is tzoffset(None, 3600) (or whichever TZ it's in) rather than a specific timezone.

By now I have a lot more changes in https://github.com/daym/django-scheduler --and that I do use with recurrent events in production (using fullcalendar in Javascript).

The ideas neccessary to fix it are the following (already used by django-scheduler but I had to find out):

  • When you specify recurring events as a human you want to specify it in local time (since it recurs at the same time in local time regardless of summer-/wintertime changes--for example: in summer, the event starts at 10:00. In winter, the event starts at 10:00 too). Since the rfc generator used for generating events only supports naive datetimes that is modeled with naive datetimes. Have to use those for the rfc generator and eventually convert them to timezone-aware at the boundaries
  • Timestamps stored in the database are stored as timestamp with time zone in postgres (which For timestamp with time zone , the internally stored value is always in UTC according to postgres docs) AND I now store a timezone string into the database table schedule_event as well (pytz, as you found out, really wants you to use named timezones--which makes sense because there can be other local variations depending on politics, so the timezone offset is not uniquely identifying what you have to do).

I'll eventually add more PRs here about it.

More details about timezones and datetime: https://blog.ganssle.io/articles/2018/03/pytz-fastest-footgun.html - all these things are not at all incorporated into this PR (it would require Python >= 3.6 if we did).

o_start = pytz.timezone(str(tzinfo)).localize(o_start) 

Generates an UnknownTimeZone exception as tzinfo is tzoffset(None, 3600) rather than an actual timezone.

Yeah :(

See above.

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

Successfully merging this pull request may close these issues.

None yet

2 participants