[Django] #30817: Document that Sitemap.items can return a QuerySet
Django
------------------------------------------------+-------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Keywords: sitemap
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+-------------------------
Currently, the Django docs don't say that `Sitemap.items()` can return a
`QuerySet`. It just says it can return a list:
> Sitemap.items: Required. A method that returns a list of objects.
> :attr:`~Sitemap.items()` is a method that returns a list of objects.
We know it can be a `QuerySet` because e.g. the items are passed to a
`Paginator`:
and `Paginator` objects accept `QuerySet` objects:
https://docs.djangoproject.com/en/2.2/topics/pagination/#example
Knowing that it can return a `QuerySet` is useful if you want to paginate
your sitemap without having to temporarily query and store in memory all
objects in one (possibly giant) list.
--
Ticket URL: <https://code.djangoproject.com/ticket/30817>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Keywords: sitemap | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by felixxm):
* stage: Unreviewed => Accepted
Comment:
`Sitemap.items()` can return not only a `QuerySet` or `list` but also
`tuple` or any other sliceable object with a `count()` or `__len__()`
method, however I don't think that we need to be so specific here. There
is also a `GenericSitemap` that allows you to create a sitemap by passing
it a dictionary which has to contain a `queryset` entry. I would suggest
to change `list` to a sequence, e.g.
{{{
A method that returns a :term:`sequence` of objects.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:1>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Christoffer Sjöbergsson):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/11855 PR
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:2>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Chris Jerdonek):
If it's going to say "sequence," I would suggest explicitly listing
`QuerySet` as an example (e.g. by adding "including `QuerySet`"
afterwards). The reason is that I don't think it's obvious whether or not
a `QuerySet` is considered a sequence, and `QuerySet` is the specific
thing I had a question about. For example, if the reader looks up Python's
definition of a sequence, it's not clear whether `QuerySet` supports all
of the "common sequence operations":
https://docs.python.org/3/library/stdtypes.html#common-sequence-operations
Like, concatenation is one of the operations, and concatenation isn't
documented in the `QuerySet` API:
https://docs.djangoproject.com/en/dev/ref/models/querysets/
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:3>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by felixxm):
`QuerySet` don't need to support these operation to be considered as a
`sequence`, please take a look at [https://docs.python.org/3/glossary.html
#term-sequence term-sequence]. Moreover changed paragraph is about "an
example" with `QuerySet`. That's why using sequence sounds sufficient to
me.
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:4>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Chris Jerdonek):
Especially since `QuerySet` is a common case, it seems better to state it
explicitly rather than leaving it as a puzzle for the reader to figure
out.
Even if the reader clicks on the glossary entry, the answer still isn't
clear and leaves a subtle, nuanced questions. For example, the glossary
entry says "an iterable which supports efficient element access using
//integer// indices," but `QuerySet` objects don't support negative
indices. So does that mean `QuerySet` objects aren't included? Rather than
the reader having to sort through questions like these, the docs can be
more helpful by simply stating it.
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:5>
Django
Reporter: Chris Jerdonek | Owner: KESHAV
Type: | KUMAR
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by KESHAV KUMAR):
* owner: nobody => KESHAV KUMAR
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner:
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by felixxm):
* owner: KESHAV KUMAR => Christoffer Sjöbergsson
Comment:
KESHAV, this issue has a patch already.
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner:
Type: | Christoffer Sjöbergsson
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by KESHAV KUMAR):
Can you tell me why it is stil open ?
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner:
Type: | Christoffer Sjöbergsson
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by felixxm):
Because patch is not merged.
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner:
Type: | Christoffer Sjöbergsson
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"7b3c06cd72e691ffd932ccce338701c37297a415" 7b3c06cd]:
{{{
#!CommitTicketReference repository=""
revision="7b3c06cd72e691ffd932ccce338701c37297a415"
Fixed #30817 -- Clarified return value of Sitemap.items().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner:
Type: | Christoffer Sjöbergsson
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"516200c09eb5664a529eced4991a599f7e7bb3c8" 516200c0]:
{{{
#!CommitTicketReference repository=""
revision="516200c09eb5664a529eced4991a599f7e7bb3c8"
[3.0.x] Fixed #30817 -- Clarified return value of Sitemap.items().
Backport of 7b3c06cd72e691ffd932ccce338701c37297a415 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner:
Type: | Christoffer Sjöbergsson
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: sitemap | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"da31472abfdc6364fcd00d6c902db8bdc04965c8" da31472a]:
{{{
#!CommitTicketReference repository=""
revision="da31472abfdc6364fcd00d6c902db8bdc04965c8"
[2.2.x] Fixed #30817 -- Clarified return value of Sitemap.items().
Backport of 7b3c06cd72e691ffd932ccce338701c37297a415 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30817#comment:12>