[Django] #32661: Make lack of timezone support in TimeField more explicit.
Django
-------------------------------------+-------------------------------------
Reporter: ab-10 | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: 3.2
layer (models, ORM) | Keywords: timezone, datetime,
Severity: Normal | datetimefield, timefield, models
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
As tzinfo is currently dropped silently from TimeField it leads to tricky
bugs for users who expect TimeField to have consistent behaviour with
DateTimeField.
This issue suggests two changes, happy to make a PR if the community
aggrees with them:
1. Mention that TimeField drops timezone in Model field reference:
https://docs.djangoproject.com/en/3.2/ref/models/fields/#timefield
2. Throw a warning when a TimeField with tzinfo is saved (similarly as
there's a warning when a naive DateTimeField is saved).
--
Ticket URL: <https://code.djangoproject.com/ticket/32661>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* cc: Aymeric Augustin (added)
* easy: 1 => 0
* stage: Unreviewed => Accepted
Comment:
> This issue suggests two changes, happy to make a PR if the community
aggrees with them:
>
> 1. Mention that TimeField drops timezone in Model field reference:
https://docs.djangoproject.com/en/3.2/ref/models/fields/#timefield
It's already [https://docs.djangoproject.com/en/dev/topics/i18n/timezones
/#naive-and-aware-datetime-objects documented] that: ''"Django only
supports naive time objects and will raise an exception if you attempt to
save an aware time object, as a timezone for a time with no associated
date does not make sense."''
> 2. Throw a warning when a TimeField with tzinfo is saved (similarly as
there's a warning when a naive DateTimeField is saved).
Django should raise an exception if you attempt to save an aware time
object (at least on
[https://github.com/django/django/blob/aa4acc164d1247c0de515c959f7b09648b57dc42/django/db/backends/oracle/operations.py#L552-L553
Oracle],
[https://github.com/django/django/blob/aa4acc164d1247c0de515c959f7b09648b57dc42/django/db/backends/mysql/operations.py#L262-L263
MySQL], and
[https://github.com/django/django/blob/aa4acc164d1247c0de515c959f7b09648b57dc42/django/db/backends/sqlite3/operations.py#L272-L273
SQLite]), however it looks that `timezone.is_aware()` doesn't work for
`datetime.time`.
{{{
>>> t = datetime.time(21, 22, 23, 240000,
tzinfo=timezone.get_current_timezone())
>>> t.tzinfo
<DstTzInfo 'America/Chicago' LMT-1 day, 18:09:00 STD>
>>> timezone.is_aware(t)
False
}}}
Maybe it's enough to check `tzinfo`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: nobody => Itay4
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Itay
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/14401 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: Itay Keren => Abhyudai
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Abhyudai):
* has_patch: 0 => 1
Comment:
new [https://github.com/django/django/pull/14583 pull request]
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:5>
Django
object even when the UTC offset isn't known.
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
I double checked, this issue is more tricky and `DateTimeField`s are also
affected. `tzinfo` subclasses may not specify the UTC offset (e.g.
`pytz.timezone("Europe/Paris")`), in such cases we need to also check
`tzinfo`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:6>
Django
object even when the UTC offset isn't known.
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:7>
Django
object even when the UTC offset isn't known.
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Regression in 432678dbc1dd4f80203468d83bb0eb6c20ed5247.
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:8>
Django
object even when the UTC offset isn't known.
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:9>
Django
object even when the UTC offset isn't known.
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => invalid
Comment:
`datetime`/`time` objects with `tzinfo` which don't declare `utcoffset`
should be treated as naive. Closing as invalid per Aymeric's
[https://github.com/django/django/pull/14583#issuecomment-873395340
comment]:
{{{
This code has been working well for over 10 years so let's be careful.
Here's my analysis.
The docs say:
> A datetime object d is aware if both of the following hold:
>
> 1. d.tzinfo is not None
> 2. d.tzinfo.utcoffset(d) does not return None
>
> Otherwise, d is naive.
This hasn't changed this Python 2.7. (I didn't check further back.)
This was explicitly the implementation before
432678dbc1dd4f80203468d83bb0eb6c20ed5247.
Also, the documentation of datetime.utcoffset, `value.utcoffset()`
literally implemented as `None if value.tzinfo is None else
value.tzinfo.utcoffset(value)`.
As a consequence of the above, each of the following propositions are
strictly equivalent:
...
Therefore I believe the change proposed here is wrong for at least three
reasons:
- Any logic changes in this area will diverge from the definition in the
Python docs.
- The proposed logic is redundant — if `value.tzinfo is None`, then
`value.utcoffset() is None`: this is the first six words of the
documentation of datetime.utcoffset: "If tzinfo is None, returns None"; I
don't see how adding this redundant logic is an improvement.
- The proposed logic is wrong — if you wanted to go back to the previous
implementation, you should use `and` instead of `or` in `is_naive`.
Finally, I believe that have two one-liner convenience functions
implemented as `value.utcoffset() is None` and `value.utcoffset() is not
None` is fine. I don't see the need for adding a level of indirection and
the overhead of a function call here.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:10>
Django
the UTC offset isn't known.
-------------------------------------+-------------------------------------
Reporter: Armin Stepanjan | Owner: Abhyudai
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: timezone, datetime, | Triage Stage: Accepted
datetimefield, timefield, models |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/32661#comment:11>