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

Add note / warning about non-existent datetimes #180

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions xocto/localtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,41 @@
from . import numbers, ranges


# Note [Non-existent datetimes]
#
# Python datetimes can be constructed for times that don't
# actually exist in certain timezones, due to clock changes
# at Daylight Savings Time (DST) boundaries.
#
# For example, 1AM on 29/02/2020 doesn't exist in GBR as the
# clocks go forwards one hour - to 2AM - at this time. But
#
# datetime(2020, 2, 29, hours=1, tzinfo=london_tz)
#
# does not error, even though it is invalid. This can cause
# problems comparing with other datetimes e.g.
#
# datetime(2020, 2, 29, hours=2, tzinfo=london_tz) -
# datetime(2020, 2, 29, hours=1, tzinfo=london_tz)
# => timedelta(hours=1) <== WRONG (should be 0)
#
# with pytest.raises(ValueError): # empty range
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ranges shouldn't be referenced in localtime - they are independent.

# FiniteDatetimeRange(
# localtime.as_utc(datetime(2020, 2, 29, hours=2, tzinfo=london_tz)),
# localtime.as_utc(datetime(2020, 2, 29, hours=1, tzinfo=london_tz)),
# )
#
# Although such datetimes are unlikely to be created directly,
# they may be created from other code in "datetime" e.g.
#
# datetime(2020, 2, 29, tzinfo=london_tz) + timedelta(hours=1)
# => datetime(2020, 2, 29, hours=1, tzinfo=london_tz)
#
# To ensure a datetime exists in its timezone, convert it
# to UTC (a timezone without DST) and back again e.g.
#
# localtime.as_localtime(localtime.as_utc(my_dt), tz=target_tz)

# Timezone aware datetime in the far future.
far_future = timezone.make_aware(datetime_.datetime.max - datetime_.timedelta(days=2))

Expand All @@ -35,6 +70,8 @@ def as_localtime(
Convert a tz aware datetime to localtime.

Wrapper for the `django.utils.timezone` function, taking the same arguments.

Warning: See Note [Non-existent datetimes]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is helpful here. The warnings will only be visible to authors of this package, not callers (who will see the warning in the doc-string.

But also, I'm not sure it has any relation to this function. Once a datetime is localised, it will be fine.

The information written is good, I just don't think it has relevance being linked in this docstring.

"""
return timezone.localtime(dt, timezone=tz)

Expand Down