[Django] #32966: time-related _check_fix_default_value() methods can be simplified
Django
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: Chris Jerdonek
Jerdonek |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I noticed that three of the `_check_fix_default_value()` method
definitions in `django/db/models/fields/__init__.py` can be simplified.
Here is one of them:
https://github.com/django/django/blob/fe074c96a343530beea50fbdd0803d3e7b739e8e/django/db/models/fields/__init__.py#L1156-L1167
For example, in each of them, `timezone.now()` is called even when the
return value isn't needed / won't be used.
--
Ticket URL: <https://code.djangoproject.com/ticket/32966>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
simplified
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:1>
Django
simplified
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:2>
Django
simplified
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Cleanup/optimization => Bug
Comment:
When I started looking at this, I noticed there is a bug on this line:
https://github.com/django/django/blob/fe074c96a343530beea50fbdd0803d3e7b739e8e/django/db/models/fields/__init__.py#L2216
It can be triggered by the following when `USE_TZ = True`:
{{{#!python
from django.db import models
import django.utils.timezone as timezone
class MyModel(models.Model):
tz = models.TimeField(default=timezone.now().timetz())
class Meta:
app_label = 'test'
field = MyModel._meta.get_field('tz')
field.check()
}}}
It results in:
{{{
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/.../django/db/models/fields/__init__.py", line 1112, in check
*self._check_fix_default_value(),
File "/.../django/db/models/fields/__init__.py", line 2220, in
_check_fix_default_value
if lower <= value <= upper:
TypeError: '<=' not supported between instances of 'datetime.datetime' and
'datetime.time'
}}}
This case is not covered in the tests here:
https://github.com/django/django/blob/fe074c96a343530beea50fbdd0803d3e7b739e8e/tests/invalid_models_tests/test_ordinary_fields.py#L743-L750
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:3>
Django
simplified
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14717
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:4>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:5>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:6>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Component: Database layer | Version: dev
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"eebebfe0a98897d55480016b3b822df60883bed9" eebebfe0]:
{{{
#!CommitTicketReference repository=""
revision="eebebfe0a98897d55480016b3b822df60883bed9"
Refs #32966 -- Added _to_naive() and _get_naive_now() for use in
DateTimeCheckMixin classes.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:8>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"6fa5d05dbaf8d7ed9ce9026202d6f8fb350fa5c4" 6fa5d05d]:
{{{
#!CommitTicketReference repository=""
revision="6fa5d05dbaf8d7ed9ce9026202d6f8fb350fa5c4"
Refs #32966 -- Simplified the _check_fix_default_value() implementations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:9>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"542e74947535b10684e9e396a0d54979e9f70fbf" 542e7494]:
{{{
#!CommitTicketReference repository=""
revision="542e74947535b10684e9e396a0d54979e9f70fbf"
Fixed #32966 -- Fixed TimeField.check() crash for timezone-aware times in
default when USE_TZ = True.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:7>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"6f5e07a84d65496abcdb876170c513c4a3840760" 6f5e07a8]:
{{{
#!CommitTicketReference repository=""
revision="6f5e07a84d65496abcdb876170c513c4a3840760"
Refs #32966 -- Refactored out DateTimeCheckMixin._check_if_value_fixed().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:10>
Django
simplified and have a bug
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
| Jerdonek
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
For future reference, it turns out the bug fixed by this ticket was
previously mentioned here, but never opened as a separate ticket:
https://code.djangoproject.com/ticket/21905#comment:19
--
Ticket URL: <https://code.djangoproject.com/ticket/32966#comment:11>