#29754: Trunc() should allow passing is_dst resolution to avoid
NonExistentTimeError/AmbiguousTimeError
-------------------------------------+-------------------------------------
     Reporter:  Alexander Holmbäck   |                    Owner:  Alexander
                                     |  Holmbäck
         Type:  New feature          |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  pytz, Trunc, is_dst  |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Alexander Holmbäck):

 * needs_better_patch:  0 => 1


Comment:

 Ok I'v been digging a bit deeper into this, here's a recap and a few
 proposals.

 When `make_aware()` is fed a datetime that isn't valid in the current
 timezone, it convert it to standard time if `is_dst=False` and daylight
 saving time if `is_dst=True`. If `is_dst=None`, which is default, it
 raises an exception. All this seems reasonable, it departs from `pytz`
 default which is to convert the invalid datetime to normal time, but
 that's justifieable and well
 
[https://docs.djangoproject.com/en/2.1/ref/utils/#django.utils.timezone.make_aware
 documented].

 In the case of `Trunc()` however, having `make_aware()` raise an excpetion
 when a truncated date is invalid is overly drastic. Hence, the current
 patch introduces a new optional flag `is_dst` to `TruncBase()` so that
 developers can decide for themselves how invalid times should be treated.
 But the consequences of not setting that flag can be dire and predicting
 those consequences would be difficult for all but the must timezone savvy
 developers (I know I wouldn't).

 That said, I've been thinking about alternative solutions.

 1) Pass `is_dst=False` to `make_aware()`. This would resonate with the
 behavior of `pytz` but it covers up invalid datetimes and a developer
 would need to re-localize them to know which datetime is valid and which
 isn't. I's also odd to explicitly choose standard time for an invalid
 datetime, even if that's what `pytz` does.

 2) Keep invalid datetimes naive. This would make invalid datetimes easier
 to distinguish from valid datetimes but as they are naive they wouldn't
 always behave like aware datetimes which could cause other problems down
 the line.

 3) Create a subclass from `pytz.tzinfo.DstTzInfo` called
 `InvalidDstTzInfo` and attach that to `tzinfo` for invalid dates. This
 would make invalid datetimes behave just like valid datetimes but also
 easy to recognize. This could be neat but it entangles django's `timezone`
 functionality with `pytz` in ways that will make it more difficult to
 maintain.

 I'm leaning slightly towards the first alternative. If we want to help
 developers checking for invalid dates we could do as `dateutil` does and
 provide `timezone.datetime_ambiguous(dt)` and
 `timezone.datetime_exists(dt)` as helper functions.

 Thoughts?

-- 
Ticket URL: <https://code.djangoproject.com/ticket/29754#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.afce1bdc8aa67bd41c9aa24ded2af20e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to