Paginator Overhall
sago
I filed a bug in paginator last week. SmileyChris pointed out that his
extra-features patch would solve the bug, and add orphan control (so
you don't get one post on its own page at the end).
I've got a pimped-up version of paginator too, and judging from
searches, it seems like lots of folk have.
So I suggest, its time to see what people need from Paginator and make
a serious joint contribution to a complete system.
My twopenneth:
1. Is there any point in having a paginator object and then having to
request data from it on a page by page basis? IMHO the use-case is:
create a query set, feed it to a paginator, tell the paginator which
page of results you want, render that page. So the paginator should
have a set_page method (related to 7 below).
2. Most page-by-page views have this data:
a) The list of objects on that page.
b) The 1-indexed number of the first and last object on the page (e.g.
results 1-10).
c) The total number of results (e.g. results 1-10 of 100).
3. I can think of many applications where it is useful to be able to
move between pages. Has next and Has previous is fine. But forums,
google &c often have a page list. It would be useful to have paginator
returning a list of pages (e.g. if pages returns 4, it would be nice to
have something returne range(1,5)). It isn't hard to trap the number of
pages and run it through range, of course, but DRY suggests this could
be in the paginator class rather than any view that needs it. This
becomes more critical with...
4. More intelligent lists of pages. In my application there may be many
tens of pages. So I do a google-like thing of having a subset of the
pages as direct links. If the user is on page 15 of 50, for example, I
might have links to pages [1,2,12,13,14,15,16,17,18,49,50]. Actually I
like to have dots to signify non-contiguous numbering. So currently I
return [1,2,None,12,13... etc], but that may just be me.
5. Orphans - see Chris' ticket.
6. Fallback to finding the number of hits with len() rather than
.count(). That single change would allow paginators to paginate any
sequence-like object, not just query sets. [As an aside Django's query
sets should probably implement __len__ as an alias for .count() to be
more 'pythonic'. Would this cause any side-effects?]
7. You should be able to get a whole dictionary of data about the page
back in one go. So I can just get my page information (consisting of
the above) and pass it right along to the template renderer. This again
is a DRY principle related to the most common use-case of getting
paginator to group objects onto a page and return the data needed to
render it. Having to pull individual bits of data out of paginator and
add them to a dictionary or to the context is very timeconsuming. It
should be a separate dictionary rather than writing to the context so
that multiple paginated lists can sit on one page easily.
Any other comments, suggestions?
And to any of the Django admins - would a substantial upgrade of this
kind to paginator be allowable, or are you locked down on its API until
v2?
Ian.
Malcolm Tredinnick
On Thu, 2006-10-12 at 13:11 -0700, sago wrote:
> All,
>
> I filed a bug in paginator last week. SmileyChris pointed out that his
> extra-features patch would solve the bug, and add orphan control (so
> you don't get one post on its own page at the end).
>
> I've got a pimped-up version of paginator too, and judging from
> searches, it seems like lots of folk have.
Every one is slightly different, no doubt, too. I am personally
reluctant to extend the Paginator object too far in any direction since
it's impossible to cover all cases. I find it better from a framework
perspective to provide the bare bones and let people sub-class for their
particular uses.
You've made a few assumptions about what you see as common use-cases in
your text below. However, the problem is that there are a lot of
different ways to use pagination and being merely the "most common" is
not sufficient because it's not a matter of everything slowly
crystallising on one alternative. So where I've challenged your
assumptions below, realise that it is because I don't like locking out
any uses. Pagination is something that can and is presented in many
different ways.
Still, most of your required functionality can be accomodated with a
couple of small additions to the Paginator class, by the looks of
things. The bulk of the customisation then remains in the user's (the
programmer's) hands.
> So I suggest, its time to see what people need from Paginator and make
> a serious joint contribution to a complete system.
>
> My twopenneth:
>
> 1. Is there any point in having a paginator object and then having to
> request data from it on a page by page basis? IMHO the use-case is:
> create a query set, feed it to a paginator, tell the paginator which
> page of results you want, render that page. So the paginator should
> have a set_page method (related to 7 below).
None of the existing paginator methods need this, so it should stay out
of the base class for that reason.
If you want to pass around the paginator instance and use it to also
store the page you are working on, you can stuff it in an attribute in
your sub-class. At that point, you will know that your methods need a
"current page" concept and so you can store it.
> 2. Most page-by-page views have this data:
> a) The list of objects on that page.
> b) The 1-indexed number of the first and last object on the page (e.g.
> results 1-10).
> c) The total number of results (e.g. results 1-10 of 100).
Some pages do operate in this way, it's true. Making it possible to work
like this is a good goal. Encouraging people to work this way is less of
a goal, since it's only one way to operate.
> 3. I can think of many applications where it is useful to be able to
> move between pages. Has next and Has previous is fine. But forums,
> google &c often have a page list. It would be useful to have paginator
> returning a list of pages (e.g. if pages returns 4, it would be nice to
> have something returne range(1,5)). It isn't hard to trap the number of
> pages and run it through range, of course, but DRY suggests this could
> be in the paginator class rather than any view that needs it. This
> becomes more critical with...
>
> 4. More intelligent lists of pages. In my application there may be many
> tens of pages. So I do a google-like thing of having a subset of the
> pages as direct links. If the user is on page 15 of 50, for example, I
> might have links to pages [1,2,12,13,14,15,16,17,18,49,50]. Actually I
> like to have dots to signify non-contiguous numbering. So currently I
> return [1,2,None,12,13... etc], but that may just be me.
Both of these are the sort of thing I would be reluctant to put into a
class like Paginator. There's just no end to the bells and whistles
department and once you walk in there, it's hard to leave.
A method for evaluating the maximum page number solves both of these
points.
If you just want a list of pages, use range() as you suggest. If you
want something fancier, again, it's entirely in your hands. There's no
"DRY" violation, because you end up writing your utility method exactly
once (it only becomes 'R' on the second and subsequent times) and you
write it to your own specifications. Trying to put in a general method
that suits everybody leads to something as complex as the calling
parameters to generic views (or worse). It becomes very hard to remember
how to use it, difficult to document and hard to maintain. At that
point, we're helping nobody.
> 5. Orphans - see Chris' ticket.
Probably worthwhile.
> 6. Fallback to finding the number of hits with len() rather than
> .count(). That single change would allow paginators to paginate any
> sequence-like object, not just query sets.
Worth considering. The current Paginator is very nicely written to
ensure it works efficiently with QuerySets, but it should be possible to
also work with arbtirary sequences.
One of Chris's patches tries to do this. I haven't reviewed it carefully
enough to know if it covers all cases yet (I'm focusing on bugs more
than enhancements when I have time these days), but if that's the size
of the changes required, it's worthwhile adding.
> [As an aside Django's query
> sets should probably implement __len__ as an alias for .count() to be
> more 'pythonic'. Would this cause any side-effects?]
>
> 7. You should be able to get a whole dictionary of data about the page
> back in one go.
This kind of assumes there is a universal concept of "the right data". I
would dispute that. This is something the client should write, since
they know what data they want. You jsut write a utility function to
return the data you want and use it wherever you need.
> So I can just get my page information (consisting of
> the above) and pass it right along to the template renderer. This again
> is a DRY principle related to the most common use-case of getting
> paginator to group objects onto a page and return the data needed to
> render it.
Your "most common" use-case isn't necessarily somebody elses. Locking
people in at the framework level is not nice.
> Having to pull individual bits of data out of paginator and
> add them to a dictionary or to the context is very timeconsuming.
Write a single function to do this and call it. Not so time consuming.
You can even sub-class Paginator and add it as a method. This provides
the callers with the ultimate control.
[...]
> Any other comments, suggestions?
>
> And to any of the Django admins - would a substantial upgrade of this
> kind to paginator be allowable, or are you locked down on its API until
> v2?
I really don't see a lot of API change needed here. Widow and orphan
support is not unreasonable. A maximum page number method is worthwhile.
Both of these are API additions, so backwards compatible.
You forgot to mention the elephant in the room, though: people are going
to want to use this functionality through generic views. So now we have
yet another parameter (or more than one) to generic views. Making it fit
nicely is not particularly easy in the code, either, once you account
for all the variations. Trying to account for everybody's particular
preferences for representing page numbers for "other pages", etc,
becomes tricky. When I last looked at Chris's enhancement, it wasn't
immediately clear to me how to resolve all those problems. You rapidly
end up needing a PaginatorFactory class or function to avoid lots of
parameters and that complicates use.
Personally, I would like the answer to be that if you want to use a
custom paginator object, then you can't use generic views. They are an
aid, not a crutch, after all. I expect that to fly like a lead balloon.
Regards,
Malcolm
sago
Yes, I agree that it is better to have a generally useful framework,
rather than an all singing and dancing solution to everybody's solution
but yours. I also agree with a lot of your post.
> Pagination is something that can and is presented in many
> different ways.
This is a subtext of my post really. I wanted to garner people's
use/expectation from the system.
Mainly because my use/expectation was so clearly different from the
facilities that it provided, and it seemed to me that I wouldn't be
alone.
If there are three common ways that people always use paginator, and
90% of people use one, and the other two are exotic, then it makes
sense (at least to me) to add a class that supports those 90%.
> Some pages do operate in this way, it's true. Making it possible to work
> like this is a good goal. Encouraging people to work this way is less of
> a goal, since it's only one way to operate.
So, I guess the question is what are the other ways ?
> > 3.
> > 4.
> There's just no end to the bells and whistles
> department and once you walk in there, it's hard to leave.
On reflection, you're probably right.
> There's no
> "DRY" violation, because you end up writing your utility method exactly
> once (it only becomes 'R' on the second and subsequent times).
The Y in DRY can be plural 'you', not just singular. If both you, and I
and a thousand other programmers are all writing functionally the same
bit of code, that is wasteful.
In the admin interface for example, Django sensibly assumes that 90% of
people will want roughly the same thing, and provides the support for
the other 10% (actually, strictly, LJW happened to have requirements
from its admin interface that 90% of the rest of us happen to share).
As long as you continue to support the other 10% its a noble aim, IMHO,
to provide for the needs of the 90%.
> > [As an aside Django's query
> > sets should probably implement __len__ as an alias for .count() to be
> > more 'pythonic'. Would this cause any side-effects?]
Anyone want to comment on this?
> > 7. You should be able to get a whole dictionary of data about the page
> > back in one go.
> This kind of assumes there is a universal concept of "the right data". I
> would dispute that.
There is already a concept of "the right data" because the class
supports querying of some data but not others (as you've said, some
bits of data you need to calculate yourself, the paginator shouldn't
provide them). Since the queries that are involved in calculating the
paginators internal data are effectively redundant (hence the
assignment to ._hits etc. in the class). Having a method to get all the
data and return it in a dictionary would be very useful and
non-controversial, surely. And it would solve the 'set_page' problem
too. Just call get_page_data(page_number) and get all the information
in a dictionary, or object.
> Your "most common" use-case isn't necessarily somebody elses. Locking
> people in at the framework level is not nice.
I agree with the second sentence. But not the first - what more common
use-case for paginator is there than grouping objects on a page? Surely
the name gives it away.
> You forgot to mention the elephant in the room, though: people are going
> to want to use this functionality through generic views.
Yes, but its a small elephant, a baby Indian, I guess.
> custom paginator object, then you can't use generic views. They are an
> aid, not a crutch, after all. I expect that to fly like a lead balloon.
I think you're right. Generic views are all about assuming things. For
example, I rarely use Generic views because most of the work I do needs
other information on the page, so even if you're looking at one db
object, I need other objects rendered in some way.
So it seems fine to me to assume that people who use generic views get
generic parameters in a generic paginator. Anyone else can write a
custom view.
Thanks for the response Malcom,
Ian.
Adrian Holovaty
> So I suggest, its time to see what people need from Paginator and make
> a serious joint contribution to a complete system.
Thanks for bringing this up. I'd like to make some big improvements to
the paginator class, and you touched on most (all?) things that I'd
like to improve...
> 1. Is there any point in having a paginator object and then having to
> request data from it on a page by page basis? IMHO the use-case is:
> create a query set, feed it to a paginator, tell the paginator which
> page of results you want, render that page. So the paginator should
> have a set_page method (related to 7 below).
I love the idea of a Paginator class that knows what page it's on. The
patch in http://code.djangoproject.com/ticket/2576 does exactly this.
I'd prefer to have it be a separate class, rather than make Paginators
optionally page-aware.
> 2. Most page-by-page views have this data:
> a) The list of objects on that page.
> b) The 1-indexed number of the first and last object on the page (e.g.
> results 1-10).
> c) The total number of results (e.g. results 1-10 of 100).
We already cover c) with paginator.hits. As for b), it'd be nice to
have a paginator.firstindex and paginator.lastindex, or some better
name. And a) is achieved by paginator.get_page().
> 3. I can think of many applications where it is useful to be able to
> move between pages. Has next and Has previous is fine. But forums,
> google &c often have a page list. It would be useful to have paginator
> returning a list of pages (e.g. if pages returns 4, it would be nice to
> have something returne range(1,5)). It isn't hard to trap the number of
> pages and run it through range, of course, but DRY suggests this could
> be in the paginator class rather than any view that needs it. This
> becomes more critical with...
Yes! I've implemented this so many times on my own that I think it
should be in the paginator class.
> 4. More intelligent lists of pages. In my application there may be many
> tens of pages. So I do a google-like thing of having a subset of the
> pages as direct links. If the user is on page 15 of 50, for example, I
> might have links to pages [1,2,12,13,14,15,16,17,18,49,50]. Actually I
> like to have dots to signify non-contiguous numbering. So currently I
> return [1,2,None,12,13... etc], but that may just be me.
I've done the [1, 2, None, 12, 13...] thing myself. Let's do this in
paginator as well.
> 5. Orphans - see Chris' ticket.
I'm not 100% sold on this, but I'll give it some more thought...
> 6. Fallback to finding the number of hits with len() rather than
> .count(). That single change would allow paginators to paginate any
> sequence-like object, not just query sets. [As an aside Django's query
> sets should probably implement __len__ as an alias for .count() to be
> more 'pythonic'. Would this cause any side-effects?]
The problem with QuerySets implementing __len__ is that sometimes you
care about the length of the QuerySet *after* the query has been done,
and sometimes you care about the number of records (i.e., *before* the
query has been done).
I like this suggestion for the paginator, though. What about one of
these solutions:
* Try .count() first. If the method doesn't exist, use len().
* Pass an optional count_method to the constructor, which defaults to
'count', but people could pass in '__len__' if they wanted. This is
slightly less elegant than the previous solution.
* Or, both!
> 7. You should be able to get a whole dictionary of data about the page
> back in one go. So I can just get my page information (consisting of
> the above) and pass it right along to the template renderer. This again
> is a DRY principle related to the most common use-case of getting
> paginator to group objects onto a page and return the data needed to
> render it. Having to pull individual bits of data out of paginator and
> add them to a dictionary or to the context is very timeconsuming. It
> should be a separate dictionary rather than writing to the context so
> that multiple paginated lists can sit on one page easily.
I think this wouldn't be necessary if we had a PaginatorPage (as
suggested in #2576), because, in theory, the PaginatorPage would have
access to all the stuff you needed in your template. Am I missing
something?
> And to any of the Django admins - would a substantial upgrade of this
> kind to paginator be allowable, or are you locked down on its API until
> v2?
The paginator class has never been documented in the official docs
(with a small exception being in a model example), so we're not locked
down on its API. And really, the types of things we're talking about
here are enhancements, not necessarily backwards-incompatibilities.
Adrian
--
Adrian Holovaty
holovaty.com | djangoproject.com
sago
Adrian Holovaty wrote:
> I love the idea of a Paginator class that knows what page it's on.
paginator. So you can still call its methods to get individual morsels
of data, or call a get_page_info method to get a PageInfo object back
with all its bits in place. Then the paginator doesn't have to be page
aware.
> > 5. Orphans - see Chris' ticket.
> I'm not 100% sold on this, but I'll give it some more thought...
I've only ever needed this once, when the paginator was paginating
(sic?) the sections of a report. In normal lists I don't mind orphans,
but if the facility was there, I think I'd probably use it, because I
rarely _need_ exactly n items on a page.
> The problem with QuerySets implementing __len__ is that sometimes you
> care about the length of the QuerySet *after* the query has been done,
> and sometimes you care about the number of records (i.e., *before* the
> query has been done).
Couldn't __len__ then do likewise: if the query hasn't been dispatched
it runs a count query, if it has it runs len on the result set.
> I like this suggestion for the paginator, though. What about one of
> these solutions:
>
> * Try .count() first. If the method doesn't exist, use len().
Exactly what I was thinking. A relatively minimal change, and very
useful.
> * Pass an optional count_method to the constructor, which defaults to
> 'count', but people could pass in '__len__' if they wanted. This is
> slightly less elegant than the previous solution.
Personally doesn't float my boat, but still very usable.
>> 7. You should be able to get a whole dictionary of data ...
> I think this wouldn't be necessary if we had a PaginatorPage (as
> suggested in #2576), because, in theory, the PaginatorPage would have
> access to all the stuff you needed in your template. Am I missing
> something?
No, that's right. To me this is the absolute biggest thing. When I want
to render a page, I want an object accessible through template methods
only (so no tedious processing in the view) that can give me everything
I need to render the links to give context and to navigate through
pages in the list. Ideally this would be an object (like
PaginatorPage), otherwise a dictionary would do.
Ian.
Afternoon
Google-style abbreviated lists. It may be a useful starting point as it
stores the page number and providers an __iter__ for displaying page
lists. Rewriting this to produce unabbreviated page lists would be
trivial.
http://django.pastebin.com/806355
FWIW, I think a Paginator that can't produce a list of page numbers is
underspecced for use in a web application. It's an extremely common
thing to do.
One of my big gripes about not having the page number in the Paginator
object is that it makes it impossible to just call has_next_page etc
from a template, a useful option when using the object_list generic
view.
SmileyChris
To clarify, I only have two patches regarding pagination:
- #2575 fixes up some of the messy bits (including a few bugs) and adds
orphaning.
- #2576 makes a PaginatorPage object which is useful for self-contained
pages (IMO usual use on a template)
> Trying to account for everybody's particular
> preferences for representing page numbers for "other pages", etc,
> becomes tricky. When I last looked at Chris's enhancement, it wasn't
> immediately clear to me how to resolve all those problems.
Neither of my patches address this (nor make it any more of a problem
than it is now).
On a side note, the one thing I dislike about the current paginator
(and my reason behind PaginatorPage) is that is seems very
un-encapsulated when it hits the template.
I'd rather have one 'paginator' key in my context (a PaginatorPage
object) than having a myriad of related keys: 'paginator_current_page',
'paginator_has_next_page', 'paginator_batch_size', ...
SmileyChris
> - #2575 fixes up some of the messy bits (including a few bugs) and adds
> orphaning.