Re: [Django] #11629: Callable QuerySet filter arguments not mentioned in documentation
Django
--------------------------------------+------------------------------------
Reporter: harrym | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by bmispelon):
* cc: bmispelon@… (added)
Comment:
The code has moved around a bit since the report but the feature still
exists:
https://github.com/django/django/blob/f403653cf146384946e5c879ad2a351768ebc226/django/db/models/sql/query.py#L1074
It's still not documented anywhere that I could find but more
interestingly, I couldn't find any tests for it either.
Note that the feature doesn't only apply to `Queryset.filter` but also to
`Queryset.exclude`, `Queryset.get`, and `Q objects`.
Interestingly, it also sorta works for `get_or_create` but only for the
get part. If the object doesn't exist in the database, then django will
set the model field's value to the callable and things will most likely
break when attempting to save to the database.
I think a good place to document this would be in the "Field lookups"
paragraph of the queryset API reference page:
https://docs.djangoproject.com/en/dev/ref/models/querysets/#id4.
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: harrym | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by manojlds@…):
I see it mentioned in the tutorials -
https://docs.djangoproject.com/en/dev/intro/tutorial05/
> Notice that we use a callable queryset argument, timezone.now, which
will be evaluated at request time. If we had included the parentheses,
timezone.now() would be evaluated just once when the web server is
started.
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: harrym | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by bmispelon):
It looks like the feature is "broken" since callable arguments are
evaluated when the queryset is built, not when it's evaluated.
This means (if I understand it right), that doing
`queryset.filter(foo=some_callable)` is basically the same as doing
`queryset.filter(foo=some_callable())`, which makes this feature less
useful.
This was discovered in ticket #20241.
I think the feature would be useful but after discussing it on IRC, it
seems it might be quite tricky to get it to work.
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: harrym | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by bmispelon):
Another ticket linked with this issue: #21632.
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: harrym | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
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:
On top of being undocumented, that feature is also untested (full test
suite still passed after removing the two lines that call the value if
it's callable).
Plus, as mentionned on #20241, this feature is going to be tricky to
implement in a useful way.
Consequently, I propose to remove the feature altogether (going through
the usual deprecation path).
Here's a PR with docs and some tests:
https://github.com/django/django/pull/2091
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: harrym | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* stage: Accepted => Ready for checkin
* component: Documentation => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: harrym | Owner: nobody
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"f1b3ab9c2158f5a7da113aef4158499ce2d42ee2"]:
{{{
#!CommitTicketReference repository=""
revision="f1b3ab9c2158f5a7da113aef4158499ce2d42ee2"
Fixed #11629 -- Deprecated callable arguments to queryset methods.
Callable arguments were an untested and undocumented feature.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: harrym | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"2cd00424c4a14b04973629d9f027df5281806e15"]:
{{{
#!CommitTicketReference repository=""
revision="2cd00424c4a14b04973629d9f027df5281806e15"
Removed support for callable QuerySet arguments per deprecation timeline;
refs #11629.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/11629#comment:12>