[Django] #33680: Documentation example of customising model instance loading has a bug
Django
-------------------------------------+-------------------------------------
Reporter: Ali-Toosi | Owner: nobody
Type: Bug | Status: new
Component: | Version: 4.0
Documentation | Keywords: documentation,
Severity: Normal | from_db, model instance loading
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
The docs provide an example of how the model instance loading can be
changed and how `_loaded_values` can be saved for future comparison here:
https://docs.djangoproject.com/en/4.0/ref/models/instances/
In this example, it checks if there are any values not loaded from db then
add them as DEFERRED values so class instantiation would work
(`cls(*values)`). However, this would mean `values` list has items now
that do not map to any `field_names` so when at the end of the function,
we zip them together and store in `_loaded_values`, the code will fail.
The easiest fix for this is to update the `field_names` too so they would
1. work and 2. show which fields were not loaded from the db at the time.
--
Ticket URL: <https://code.djangoproject.com/ticket/33680>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Ali-Toosi | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: documentation, | Triage Stage:
from_db, model instance loading | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Old description:
> The docs provide an example of how the model instance loading can be
> changed and how `_loaded_values` can be saved for future comparison here:
> https://docs.djangoproject.com/en/4.0/ref/models/instances/
>
> In this example, it checks if there are any values not loaded from db
> then add them as DEFERRED values so class instantiation would work
> (`cls(*values)`). However, this would mean `values` list has items now
> that do not map to any `field_names` so when at the end of the function,
> we zip them together and store in `_loaded_values`, the code will fail.
>
> The easiest fix for this is to update the `field_names` too so they would
> 1. work and 2. show which fields were not loaded from the db at the time.
New description:
The docs provide an example of how the model instance loading can be
changed and how `_loaded_values` can be saved for future comparison here:
https://docs.djangoproject.com/en/4.0/ref/models/instances/
In this example, it checks if there are any values not loaded from db then
add them as DEFERRED values so class instantiation would work
(`cls(*values)`). However, this would mean `values` list has items now
that do not map to any `field_names` so when at the end of the function,
we zip them together and store in `_loaded_values`, the code will fail.
The easiest fix for this is to update the `field_names` too so they would
1. work and 2. show which fields were not loaded from the db at the time.
I've opened a PR for this here:
https://github.com/django/django/pull/15664
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Documentation | Version: 4.0
Severity: Normal | Resolution:
Keywords: documentation, | Triage Stage:
from_db, model instance loading | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Ali Toosi
* needs_better_patch: 0 => 1
* status: new => assigned
Comment:
Thanks for the ticket.
> The easiest fix for this is to update the field_names too so they would
1. work and 2. show which fields were not loaded from the db at the time.
I'd keep the previous behavior and rather filter out `DEFERRED` from the
list of values, e.g.
{{{#!diff
diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt
index 3638c6ccff..ec0232673e 100644
--- a/docs/ref/models/instances.txt
+++ b/docs/ref/models/instances.txt
@@ -103,7 +103,9 @@ are loaded from the database::
instance._state.adding = False
instance._state.db = db
# customization to store the original field values on the
instance
- instance._loaded_values = dict(zip(field_names, values))
+ instance._loaded_values = dict(
+ zip(field_names, (value for value from values where value is
not DEFERRED))
+ )
return instance
def save(self, *args, **kwargs):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Documentation | Version: 4.0
Severity: Normal | Resolution:
from_db, model instance loading |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:3>
Django
-------------------------------------+-------------------------------------
Component: Documentation | Version: 4.0
Severity: Normal | Resolution:
Keywords: documentation, | Triage Stage: Accepted
from_db, model instance loading |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ali Toosi):
Replying to [comment:2 Mariusz Felisiak]:
Thanks, Mariusz. I've updated the PR with your suggestion.
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Ali Toosi | Owner: Ali Toosi
Type: Bug | Status: assigned
Component: Documentation | Version: 4.0
Severity: Normal | Resolution:
from_db, model instance loading | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Ali Toosi | Owner: Ali Toosi
Component: Documentation | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: documentation, | Triage Stage: Ready for
from_db, model instance loading | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"faab9e6769b01c18d9e3a31504601452eede6150" faab9e67]:
{{{
#!CommitTicketReference repository=""
revision="faab9e6769b01c18d9e3a31504601452eede6150"
Fixed #33680 -- Corrected example of customizing model loading in docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Ali Toosi | Owner: Ali Toosi
Type: Bug | Status: closed
Component: Documentation | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: documentation, | Triage Stage: Ready for
from_db, model instance loading | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"8b2a93ee5bec74e0b4d84bd5c6a22c62b157232e" 8b2a93ee]:
{{{
#!CommitTicketReference repository=""
revision="8b2a93ee5bec74e0b4d84bd5c6a22c62b157232e"
[4.0.x] Fixed #33680 -- Corrected example of customizing model loading in
docs.
Backport of faab9e6769b01c18d9e3a31504601452eede6150 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33680#comment:7>