It is real to add ticket #8054 to 1.3 milestone?

9 views
Skip to first unread message

alekam

unread,
Oct 12, 2010, 12:47:21 PM10/12/10
to Django developers
Hi All,

I found very useful ticket #8054. This ticket has accepted status and
assigned to brosher about 2 years. The problem describes on ticket
detail page and in the wiki http://code.djangoproject.com/wiki/ListColumns
The ticket has patch witch passes all existing regression tests and I
plan to write unit tests for it.
It is real to add ticket #8054 to 1.3 milestone?


Cheers,

Alex Kamedov

Gabriel Hurley

unread,
Oct 12, 2010, 4:12:03 PM10/12/10
to Django developers
Hi alekam,

This is certainly the kind of ticket that we want to deal with in 1.3
since it's been around so long and deferred a couple times. Starting
the discussion here is definitely the right way to make it happen.

That said, at the very least it needs a thorough code review, a full
set of tests, and good documentation before it can be included.

I'm not personally sold on the way the functionality is implemented,
but I'd have to think on what would be preferable. I'm sure others
will have stronger opinions on it one way or the other.

All the best,

- Gabriel

On Oct 12, 9:47 am, alekam <kame...@gmail.com> wrote:
> Hi All,
>
> I found very useful ticket #8054. This ticket has accepted status and
> assigned to brosher about 2 years. The problem describes on ticket
> detail page and in the wikihttp://code.djangoproject.com/wiki/ListColumns

Alex Kamedov

unread,
Oct 13, 2010, 8:35:47 AM10/13/10
to django-d...@googlegroups.com
Hi All,

I uploaded patch with tests and little improvements 
I think it ready for review.

I really sorry, but I couldn't write good documentation because English isn't my own language. The maximum of my possibility in writing docs is here http://code.djangoproject.com/wiki/ListColumns   For more examples see folder admin_listcolumn in regression tests provided by last my patch.

Cheers,

Alex Kamedov

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.


Alex Kamedov

unread,
Oct 26, 2010, 12:23:40 PM10/26/10
to django-d...@googlegroups.com
Can anybody rewiew the patch?

bur...@gmail.com

unread,
Oct 27, 2010, 12:58:19 AM10/27/10
to django-d...@googlegroups.com
Hi Alex,

Patch is looking good, except few small things.
Wiki docs are also very good, but they are quite incomplete.
Replied to the ticket.

--
Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov,
MSN: bu...@live.com

Mikhail Korobov

unread,
Oct 27, 2010, 6:45:12 AM10/27/10
to Django developers
Hi Alex and Yuri,

To make it clear: is this only a syntax change and all other benefits
can be achieved with current implementation?

The benefits from the wiki:

1. ability to customize and localize 3rd-party application without
fork it - it is as easy with current ModelAdmin. ModelAdmin looks up
it's own methods so 'get_bar_column' in wiki can be put into
AccountAdmin.

2. ability to apply custom template filters on field value or model
method returned value without any magic - it is also possible:
template filters are just functions and @register.filter decorator
doesn't harm them.

3. ability to specify witch field will be used in order for column
based on model method - this is also totally possible with current
implementation

4. beautifully API to admin change list customization - as I can see
it is the only benefit here

Am I correct and the goal is only to change list_display syntax?


On 27 окт, 10:58, "burc...@gmail.com" <burc...@gmail.com> wrote:
> Hi Alex,
>
> Patch is looking good, except few small things.
> Wiki docs are also very good, but they are quite incomplete.
> Replied to the ticket.
>
>
>
>
>
> On Tue, Oct 26, 2010 at 11:23 PM, Alex Kamedov <kame...@gmail.com> wrote:
> > Can anybody rewiew the patch?
>
> > On Wed, Oct 13, 2010 at 6:35 PM, Alex Kamedov <kame...@gmail.com> wrote:
>
> >> Hi All,
> >> I uploaded patch with tests and little improvements
>
> >>http://code.djangoproject.com/attachment/ticket/8054/8054-list-column...

Russell Keith-Magee

unread,
Oct 27, 2010, 8:07:12 AM10/27/10
to django-d...@googlegroups.com

I've looked at the wiki and ticket, and I'm having difficulty seeing
what benefits this proposal offers. It looks to me like you're
proposing to change the syntax that is used to declare list_display,
but I can't really see any signficant changes in the flexibility of
the new approach. As far as I can make out, everything you describe
(and more) can already be achieved by using callables in list_display.

This patch isn't going to get added to trunk just so we can add a new
syntax for things we can already do. It needs to clearly add new
capabilities that don't currently exist.

Yours,
Russ Magee %-)

bur...@gmail.com

unread,
Oct 27, 2010, 8:30:07 AM10/27/10
to django-d...@googlegroups.com
Hi Mikhail,

On Wed, Oct 27, 2010 at 5:45 PM, Mikhail Korobov <kmi...@googlemail.com> wrote:
> Hi Alex and Yuri,
>
> To make it clear: is this only a syntax change and all other benefits
> can be achieved with current implementation?
>
> The benefits from the wiki:
>
> 1. ability to customize and localize 3rd-party application without
> fork it - it is as easy with current ModelAdmin. ModelAdmin looks up
> it's own methods so 'get_bar_column' in wiki can be put into
> AccountAdmin.
>
> 2. ability to apply custom template filters on field value or model
> method returned value without any magic - it is also possible:
> template filters are just functions and @register.filter decorator
> doesn't harm them.
>
> 3. ability to specify witch field will be used in order for column
> based on model method - this is also totally possible with current
> implementation
>
> 4. beautifully API to admin change list customization - as I can see
> it is the only benefit here
>
> Am I correct and the goal is only to change list_display syntax?

Yes, I don't see any other goal either, so I have controversial
feelings about the issue.
It has only syntax sugar, but that sugar is very sweet.

Alex Kamedov

unread,
Oct 28, 2010, 3:04:05 AM10/28/10
to django-d...@googlegroups.com
Hi All. Big thanks to response!

> Am I correct and the goal is only to change list_display syntax?
Yes, I don't see any other goal either, so I have controversial
feelings about the issue.
It has only syntax sugar, but that sugar is very sweet.
Hm. Yes, it provide only syntax sugar. But very often it'll be more easiest way to customize admin (Eg.: apply template tags to model fields and existing model methods).

Patch and wiki page were updated according to Yuri Baburov (buriy) recomendations.

Russell Keith-Magee

unread,
Oct 28, 2010, 9:47:39 PM10/28/10
to django-d...@googlegroups.com
On Thu, Oct 28, 2010 at 3:04 PM, Alex Kamedov <kam...@gmail.com> wrote:
> Hi All. Big thanks to response!
>>
>> > Am I correct and the goal is only to change list_display syntax?
>> Yes, I don't see any other goal either, so I have controversial
>> feelings about the issue.
>> It has only syntax sugar, but that sugar is very sweet.
>
> Hm. Yes, it provide only syntax sugar. But very often it'll be more easiest
> way to customize admin (Eg.: apply template tags to model fields and
> existing model methods).

... which is something you can already do with a callable column::

class MyModelAdmin(ModelAdmin):
def formatted_foobar(self, obj):
return slugify(truncatewords(obj.foobar, 2))

list_display = ('formatted_foobar', 'name', 'age')

with the added benefit that you can do a lot more that just applying a
template filter -- to pick a trivial example, you could conditionally
apply *different* template filters, depending on the value of the
attribute.

Honestly, I can't see the benefit in what you're proposing here.
Before you spend a whole lot more time updating the patch to try and
make and keep it trunk ready, you need to convince me (or another
member of the core team) that the idea you're proposing is worth
adding to trunk. Otherwise you're just going to be spending a lot of
time maintaining a patch that won't ever get committed.

Yours,
Russ Magee %-)

Alex Kamedov

unread,
Nov 4, 2010, 12:33:30 AM11/4/10
to django-d...@googlegroups.com
Hi All! Sorry for late response.


On Fri, Oct 29, 2010 at 7:47 AM, Russell Keith-Magee <rus...@keith-magee.com> wrote:
... which is something you can already do with a callable column::

class MyModelAdmin(ModelAdmin):
  def formatted_foobar(self, obj):
      return slugify(truncatewords(obj.foobar, 2))

   list_display = ('formatted_foobar', 'name', 'age')

with the added benefit that you can do a lot more that just applying a
template filter -- to pick a trivial example, you could conditionally
apply *different* template filters, depending on the value of the
attribute.
Yes, I know about it. Ticket #8054 is not cover this case, you need to define a new function for this. Ticket #8054 is not about it. It's about how you put in ModelAdmin meta data of this function related to display it in admin change list. I'll try to explain it bellow, but now let's speak about template filters. Most common case in change list customization is then you need just apply some template filters to 1 model field for every model objects. This ticket lets do it more easy. If you need more logic to render field, you must define new callable for it of course.

The main benefits of #8054 is considerate all options related to admin change list customization in list_display property of ModelAdmin. 

It solves some little code organization problems with callable attributes. For example, If you want customize change list header for column getting form model method, you can use attribute short_description for it, but this is related only for admin change list and not ever need anything else. It's more logically to define it something in ModelAdmin and not creating new callable or copy existing for customization needs. It's first proposed benefits.

The second is a small refactoring of django.contrib.admin. This ticket adds some changes to admin change list generation. Currently admin change list is generating with a couple of template tags and most of all change list logic considerate in this template tags. This ticket move logic of displaying object field value and column header to special place - classes inherited from ListColumn.


Honestly, I can't see the benefit in what you're proposing here.
Before you spend a whole lot more time updating the patch to try and
make and keep it trunk ready, you need to convince me (or another
member of the core team) that the idea you're proposing is worth
adding to trunk. Otherwise you're just going to be spending a lot of
time maintaining a patch that won't ever get committed.
Hm. In this case why this ticket is on accepted triage stage and assigned on the one from core team developers a long time?
It is my first experience to send my code in this community, this ticket was related to my needs, and I followed only instructions from official Django documentation.


Cheers,

Alex Kamedov

Russell Keith-Magee

unread,
Nov 4, 2010, 1:35:17 AM11/4/10
to django-d...@googlegroups.com

I can see that you have done what you have done in good faith.
However, I think you have misread the tone around the ticket.

The problem is that the state of a ticket in Trac can only be used as
an indication, not as an absolute guide. We allow anyone to modify
tickets, so just because a ticket is "accepted" doesn't necessarily
mean that the idea has been accepted -- you need to pay attention to
*who* has accepted the ticket. Tickets also decay over time; an idea
that seemed great and was accepted a year ago may not be accepted
today, because of other changes that have occurred in Django. Lastly,
(and most applicable to this case), a ticket that has been partially
addressed may not be updated to reflect an accurate state of the
ticket.

Looking at #8054 specifically: The ticket was accepted on 13 August
2008, after an initial period in design decision needed. The design
discussion entirely revolves around the idea of methods/functions as
list_display items, and, on 14 August 2008 (r8352), that's exactly
what Brian committed.

Brian followed that checkin with a comment: "Moving to post-1.0 for
the API changing parts. This should be looked at with a ChangeList and
templatetag refactor of the admin. Not sure when or how, but this is
related."

Since that time, there has been no core team member involvement or
discussions. I'm not aware of any django-dev discussions about the
feature. However, there have been a couple of people maintaining the
original patch -- the one that Brian decided *not* to apply in the
first place.

Reading between the lines, this means that the ticket really isn't in
an accepted state anymore -- at the very least, it's DDN, and probably
wontfix. I don't know what Brian had in mind at the time, but he
obviously wasn't happy with the proposed approach, and two years
later, I agree.

> It is my first experience to send my code in this community, this ticket was
> related to my needs, and I followed only instructions from official Django
> documentation.

First off -- what you have encountered with this ticket isn't my idea
of an ideal first contribution experience. I can see that you have
done everything that the contribution guide said to do -- it's just
that the contribution guide doesn't do a good job of capturing the
esoteric nature of "reading" Trac tickets. In that respect, we (the
core team) have failed you. I wholeheartedly apologize for that.

I suppose the only practical advice I can give would be the following:

* Trac isn't an absolute; the context is just as important as the
literal message. You need to take into account who says things, when
they said them, and the completely ambiguous who *hasn't* said things.

* If you're going to engage in a big project, it helps to make sure
that your idea has support first. This means getting someone to
confirm that a bug is real before you spend a whole lot of time fixing
the issue, and confirming that the core team currently supports a
proposed feature before you implement it.

These are both points that the contributions guide doesn't do a good
job of reinforcing, and it probably should. Ticket #14401 is a work in
progress discussing a new "contributions howto" that would cover some
of the fuzzy issues around contributing to Django; I've just updated
the draft text to cover the points I've raised here.

Yours,
Russ Magee %-)

Alex Kamedov

unread,
Nov 4, 2010, 2:56:58 AM11/4/10
to django-d...@googlegroups.com
I've tried communicate with Brian via email before start discussion here. I take the email address from his github account. In email I ask he about his plans on this ticket, but don't get reply.

Originally I need to add some customization to admin change list requested by my clients. It's possible with some ways with current django.contrib.admin and I've need to found of the best of them. It's not related with template tags or something related with ticket #8054. To resolve my problem I have need to know about how admin change list is rendered. I thought, reviewing Daniel's patch is a short way to understand it. I like his idea. I port his patch on more newer version of Django,  share it with community via Track and after some time from unfortunately communication with Brian try to promote it here.

I don't know that I spent my time on no purpose. Anyway I've successfully done all my goals and in addiction learned about how django test suit is organized. If this ticket isn't useful for Django today, so this is life.



Yours,
Russ Magee %-)

Reply all
Reply to author
Forward
0 new messages