Hi Ian, 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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django users" group. To post to this group, send email to django-users@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-users -~----------~----~----~----~------~----~------~--~---
