[Django] #30947: Apply data structure best practices to the django.contrib models
Django
------------------------------------------------+------------------------
Reporter: Jon Dufresne | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
In PR https://github.com/django/django/pull/11267 an argument is presented
that explains why `list` is often the best choice for Model.Meta data
structures. This landed as
https://github.com/django/django/commit/97d3321e89c8d4434927bdbc308db1ccffa99d3b.
Now change all models in `django.contrib` to follow our own stated best
practices. People often look at existing examples for how to write new
code, so we might as well promote the standards we document.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Hoppe):
* status: new => assigned
* cc: Johannes Hoppe (added)
* component: Uncategorized => Documentation
* owner: nobody => Jon Dufresne
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:1>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:2>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* needs_better_patch: 1 => 0
Comment:
Updated PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:3>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by CarlosMirdeSouza):
* stage: Accepted => Ready for checkin
Comment:
It`s ok.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:4>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b9fe7f9294b1b4fc974c008adeb96e1375cdb0c6" b9fe7f92]:
{{{
#!CommitTicketReference repository=""
revision="b9fe7f9294b1b4fc974c008adeb96e1375cdb0c6"
Fixed #30947 -- Changed tuples to lists in model Meta options in
django.contrib modules.
The Django "Model Meta options" docs provide examples and generally
point the reader to use lists for the unique_together and ordering
options. Follow our own advice for contrib models.
More generally, lists should be used for homogeneous sequences of
arbitrary lengths of which both unique_together and ordering are.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:6>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Component: Documentation | Version: master
Severity: Normal | Resolution:
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne <jon.dufresne@…>):
In [changeset:"e5cacb1f47cb3a2943bbc7685a630c84503b0d1b" e5cacb1]:
{{{
#!CommitTicketReference repository=""
revision="e5cacb1f47cb3a2943bbc7685a630c84503b0d1b"
Refs #30947 -- Changed tuples to lists in model Meta options examples in
docs.
Follow up to 97d3321e89c8d4434927bdbc308db1ccffa99d3b.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:5>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Component: Documentation | Version: master
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:"2000ed5180308ae844f3df7e4ed1be64e163755b" 2000ed51]:
{{{
#!CommitTicketReference repository=""
revision="2000ed5180308ae844f3df7e4ed1be64e163755b"
[3.0.x] Refs #30947 -- Changed tuples to lists in model Meta options
examples in docs.
Follow up to 97d3321e89c8d4434927bdbc308db1ccffa99d3b.
Backport of e5cacb1f47cb3a2943bbc7685a630c84503b0d1b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:7>
Django
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Cleanup/optimization | Status: closed
Component: Documentation | Version: master
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:"a7f90d9c02f5ed6f6dc1621ef10b9db1f042600b" a7f90d9]:
{{{
#!CommitTicketReference repository=""
revision="a7f90d9c02f5ed6f6dc1621ef10b9db1f042600b"
[2.2.x] Refs #30947 -- Changed tuples to lists in model Meta options
examples in docs.
Follow up to 97d3321e89c8d4434927bdbc308db1ccffa99d3b.
Backport of e5cacb1f47cb3a2943bbc7685a630c84503b0d1b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Cleanup/optimization | Status: closed
Component: Documentation | Version: master
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):
#30947 was a duplicate for extra docs changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Jon
Type: | Dufresne
Component: Documentation | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: closed => new
* has_patch: 1 => 0
* resolution: fixed =>
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: Jon Dufresne | Owner: (none)
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Jon Dufresne => (none)
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:11>
Django
--------------------------------------+------------------------------------
Reporter: Jon Dufresne | Owner: (none)
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:12>
Django
--------------------------------------+------------------------------------
Reporter: Jon Dufresne | Owner: (none)
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:13>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
* owner: (none) => rihib
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:14>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by rihib):
* owner: rihib => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:15>
Django
-------------------------------------+-------------------------------------
Type: | Gupta
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Ojas Gupta):
* owner: (none) => Ojas Gupta
* status: new => assigned
Comment:
Ok so I have to convert tuple of list to list of list.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Ojas
Type: | Gupta
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Alex Morega):
This is actually a pet peeve of mine; would love to see the docs updated
to using lists instead of tuples.
Would this be a good checklist of places to update in the codebase and
docs? https://gist.github.com/mgax/32318118a71ce602a00a80daeecb60fc
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Ojas
Type: | Gupta
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Some of the examples you pointed should remain tuples as they are mixing
heterogenous data and mostly used to a define a schema over data.
Since we're on the topic of data structure best practices
`unique_together` and friends should likely be defined as `set` and not
`list` as only unique entries will be considered and because the order in
which they are defined on the model doesn't have any significance.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Ojas
Type: | Gupta
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Alex Morega):
> Some of the examples you pointed should remain tuples as they are mixing
heterogenous data and mostly used to a define a schema over data.
Right! I meant it as a starting point of places that might need fixing.
> Since we're on the topic of data structure best practices
`unique_together` and friends should likely be defined as `set` and not
`list` as only unique entries will be considered and because the order in
which they are defined on the model doesn't have any significance.
I see your point, but AFAIU, a side effect of `unique_together` is
creating composite database indexes, where the
[https://www.postgresql.org/docs/current/indexes-multicolumn.html order of
columns matters].
There hasn't been any activity on the ticket for 3 weeks; may I take
ownership?
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Ojas
Type: | Gupta
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
> I see your point, but AFAIU, a side effect of unique_together is
creating composite database indexes, where the order of columns matters.
The order of columns within an index or unique constraints matter, but the
order of constraints doesn't. This obviously means that you can't use the
''flattened'' syntax `index_together = {'foo', 'bar'}` but favor
`index_together = {('foo', 'bar')}`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:20>
Django
-------------------------------------+-------------------------------------
Type: | Morega
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: Ojas Gupta => Alex Morega
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16003 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Alex
Type: | Morega
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Alex
Type: | Morega
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"de6c9c70549010fc39509f9ef3f6a62ada870318" de6c9c7]:
{{{
#!CommitTicketReference repository=""
revision="de6c9c70549010fc39509f9ef3f6a62ada870318"
Refs #30947 -- Changed tuples to lists where appropriate.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: Jon Dufresne | Owner: Alex
Type: | Morega
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/30947#comment:24>