Re: Adding aggregates to ModelAdmin.list_display
I see much value in aggregating on a single page (rather than the entire
QS).
Check out this
example:
https://cloud.githubusercontent.com/assets/1499307/18555738/964622e0-7b71-11e6-9a53-3a525ba25b4b.png
Also, my current implementation (in the pull request) does support a
user-friendly name for the field, like so:
class EmployeeAdmin(admin.ModelAdmin):
list_display = ('name', 'age', 'formatted_monthly_salary')
list_aggregates = (('age', Avg), ('formatted_monthly_salary', Sum))
def formatted_monthly_salary(self, employee_or_salary):
salary = getattr(employee_or_salary, 'monthly_salary',
employee_or_salary)
return '${0:,.2f}'.format(salary)
formatted_monthly_salary.admin_aggregate_field = 'monthly_salary'
On Thursday, September 22, 2016 at 8:52:20 PM UTC+3, Andrew Mosson wrote:
>
> We have an implementation of both annotations in *list_display* and
> adding an aggregates for the entire list (which we call *list_summary*
> and what you are calling here *list_aggregates*) and there are a bunch of
> subtleties in the interaction due to Admin's built in support for
> pagination and filtering
>
> To demonstrate, let's use a simplified use case
> assume models such as Author -< Book >- Genre (Book has FKs to both
> Author and Genre)
>
> and and admin such as (as suggested in Stack Overflow post that Tim
> referenced (already possible to some extent
>
> )
>
> class AuthorAdmin(admin.ModelAdmin):
> list_display = ('author_name', 'book_count', 'book_sum_price')
> list_filters = ('book__genre__name', )
> list_summary = (('Total Books', Sum('book_count'), ),
> ('Total Book Price', Sum('book_sum_price'), ))
>
> def get_queryset(self, request):
> return super(BookAdmin, self).get_queryset(request).annotate(
> books_count=Count('books__name'),
> books_sum_price=Sum('books__price'))
>
> With regards to *list_summary* (or *list_aggregates* in the PR) our
> implementation summarizes the entire QuerySet not just a single page (my
> quick read of the patch seems to indicate that list_aggregates only
> aggregates a single page of the qs). From my perspective summarizing a
> single page doesn't provide as much value summarizing the entire QS. If
> one agrees with that then feature will would have to support a user
> friendly name for the field (implemented here as a tuple - as suggested by
> Jim). Additionally, if the feature summarizes the entire qs then the
> output should likely go above the table rather than as summary below (if it
> were below and the results were paginated, it would likely confuse the
> users)
>
> With regards to extending list_display to support annotations there are
> subtle interactions between admin, annotated query sets and filters.
>
> The qs that Django executes when the user has a filter looks like
>
> Author.objects.annotate(...).filter(...)
>
> In the code shown above this will produce the correct result set, but
> because annotate and filter aren't commutative
> ,
>
> the generated SQL ends up joining the Books table twice. Additionally,
> there are probably some cases where complex filters will give unexpected
> results.
>
> Give that the user really wants
>
> Author.objects.filter(...).annotate(...)
>
> we ended up adding a modelAdmin.get_annotations(...) method and
> subclassing ChangeList to implement this feature.
>
> One additional note is that annotations require implementing a method on
> the modelAdmin class for each annotations which seems very boilerplate-ish.
> We have extended *list_display* to automatically handle the boilerplate
> as well.
>
> Since we use this extensively these features in our applications, we would
> be excited to see them implemented as part of admin. We'd be happy to
> contribute here and if this seems like its worth pursuing I'll ask our team
> to look into refactoring the work we've done so that it could live in core
> rather than as sub-classes.
>
> On Wednesday, September 21, 2016 at 6:17:27 PM UTC-7, Josh Smeaton wrote:
>>
>> I think I'm OK with `list_aggregates` because it implies a terminal
>> queryset method which really restricts the members used to create that
>> aggregation (the GROUP BY). Adding aggregates to existing list_display
>> would require something *else* to refine the group by using `values()`.
>>
>> If list_aggregates is a useful feature, then this sounds like an
>> appropriate way to implement that. Regular annotations could be added and
>> processed within list_display, provided list_display was modified to accept
>> expressions (either aggregates or regular annotations) in tuple form for
>> alias support.
>>
>> list_aggregates -> queryset.aggregate()
>> list_display -> queryset.anno
Fellow Report - September 24, 2016
I spent a few hours this week trying to develop a better understanding of the ORM by investigating some issues [0]. Along the way, I reviewed the coverage report for the relevant areas and submitted a few PRs to remove unused code. I hope I’ll have time to continue this effort. A nice outcome might be to develop some contributor-facing documentation to ease understanding and contributing to the ORM. Let me know if you have ideas about that or would be interested in helping with that. [0] https://github.com/django/django/pull/6267 / https://github.com/django/django/pull/7289 Triaged --- https://code.djangoproject.com/ticket/27239 - Unexpected behavior on get_FIELDNAME_display when as int as value (duplicate) https://code.djangoproject.com/ticket/27242 - Add get_object_or_none to django.shortcuts (duplicate) https://code.djangoproject.com/ticket/27225 - Cache-Control's max-age doesn't match Expires for responses taken from cache (accepted) https://code.djangoproject.com/ticket/27238 - Disable check_pattern_startswith_slash if settings.APPEND_SLASH=False (accepted) https://code.djangoproject.com/ticket/27234 - Clarify the type of the django.server logger's 'request' extra context (accepted) https://code.djangoproject.com/ticket/27246 - Factor out input template in ClearableFileInput and document template class attributes (wontfix) https://code.djangoproject.com/ticket/27245 - can't revert migration with index_together with one field (wontfix) https://code.djangoproject.com/ticket/27231 - Initialize forms in ModelAdmin like View (i.e. add get_form_kwargs to contrib.admin) (wontfix) https://code.djangoproject.com/ticket/27248 - Running migrations one-by-one fails after adding django.contrib.sites to INSTALLED_APPS. (fixed) https://code.djangoproject.com/ticket/27253 - Use assertIsInstance() in test_force_text_lazy (invalid) https://code.djangoproject.com/ticket/27255 - Change test runner to display full dotted name of test (duplicate) https://code.djangoproject.com/ticket/27249 - IntegrityError when using ManyToManyField.add() with a value of incorrect type (duplicate) https://code.djangoproject.com/ticket/27260 - Performance Issue because of LEFT OUTER JOIN instead the better INNER JOIN (invalid) https://code.djangoproject.com/ticket/27240 - Allow passing custom parameters to formset forms in admin (duplicate) https://code.djangoproject.com/ticket/27259 - ManyToOneRel.name uses relatemodelname instead of relatedmodelname_set (invalid) https://code.djangoproject.com/ticket/27226 - Remove patch_response_headers()'s setting of the "Last-Modified" header (accepted) https://code.djangoproject.com/ticket/27263 - Allow validators to short-circuit in form field validation (Someday/Maybe) https://code.djangoproject.com/ticket/13161 - Avoid global registration of psycopg2 adapters/extensions (wontfix) https://code.djangoproject.com/ticket/9394 - Reverse relation lookups with a multi-table inherited model produces extraneous queries (duplicate) https://code.djangoproject.com/ticket/25049 - Oracle fails to change unique_together constraints due to case sensitivity (duplicate) https://code.djangoproject.com/ticket/27265 - Using @admin.register causes failure when AdminModel constructor is overriden (invalid) Authored https://github.com/django/django/pull/7268 - Refs #27025 -- Fixed ArrayField querying on Python 3.6. https://github.com/andialbrecht/sqlparse/pull/292 - Fix test failure on Python 3.6. (later reverted as unnecessary, as a cpython ticket I opened resulted in cpython fixing the backwards-incompatibility.) Reviewed/committed -- https://github.com/django/django/pull/7241 - Fixed #27219 -- Changed cx_Oracle client encoding to AL32UTF8 to allow 4-byte characters. https://github.com/django/django/pull/7269 - Fixed #27238 -- Disabled check_pattern_startswith_slash if settings.APPEND_SLASH. https://github.com/django/django/pull/6735 - Fixed #26610 -- Added CitextField to contrib.postgres. https://github.com/django/django/pull/7266 - Fixed #27158 -- Fixed positioning of admin's many-to-many widgets in rtl.css. https://github.com/django/django/pull/7271 - Fixed #26210 -- Prevented SMTP backend from trying to send mail after a connection failure. https://github.com/django/django/pull/7221 - Fixed #27159 -- Prevented pickling a query with an __in=inner_qs lookup from evaluating inner_qs. https://github.com/django/django/pull/7255 - Fixed #27118 -- Made QuerySet.get_or_create()/update_or_create() error for a non-field in their arguments. Reviews of core dev work https://github.com/django/django/pull/7264 - Fixed #27056 -- Allowed migrating geometry field dimension on PostGIS https://github.com/django/django/pull/7279 - Fixed #27257 -- Fixed builtin text lookups on JSONField keys. -- You received this message because you are subscribed to the Google Groups "Django developers (Contrib
