Account Options

  1. Sign in
The old Google Groups will be going away soon, but your browser is incompatible with the new version.
Google Groups Home
« Groups Home
list_display does not allow foreign keys __ syntax
There are currently too many topics in this group that display first. To make this topic appear first, remove this option from another topic.
There was an error processing your request. Please try again.
flag
  10 messages - Collapse all  -  Translate all to Translated (View all originals)
The group you are posting to is a Usenet group. Messages posted to this group will make your email address visible to anyone on the Internet.
Your reply message has not been sent.
Your post was successful
 
From:
To:
Cc:
Followup To:
Add Cc | Add Followup-to | Edit Subject
Subject:
Validation:
For verification purposes please type the characters you see in the picture below or the numbers you hear by clicking the accessibility icon. Listen and type the numbers you hear
 
Anshuman Aggarwal  
View profile  
 More options May 17 2011, 7:17 am
From: Anshuman Aggarwal <anshu...@brillgen.com>
Date: Tue, 17 May 2011 16:47:53 +0530
Local: Tues, May 17 2011 7:17 am
Subject: list_display does not allow foreign keys __ syntax
I know this has been discussed (not sure to what depth) but callables is not the solution. Please bear with me and help me (not force me) to understand why what I am suggesting is wrong.
I did look in the list and have tried taking this discussion to the ticket system. http://code.djangoproject.com/ticket/5863

In a nutshell, list_display does not accept foreign key __ syntax which is being used everywhere (including list_filter as of 1.3)...Just because callables exist as an advanced way of doing custom items in list does not take away from the argument that __ fk is a very very common requirement in the list display.

Here are the options currently available:

1. Write functions everywhere (this involves a lot of repetition for every single foreign field)
2. Write a wrapper like @lukeplant suggested in #5863 (even though it is far superior solution to individual functions in every model)

These are all HUGE violation of DRY principles on which sensible programming is based toady.
Most model fields already have a verbose_name, admin_sort_order etc. defined ...why should it be required to specify in just boilerplate code all of this all over again? Even in the wrapper function by @lukeplant, this verbose_name cannot be retrieved from the related model.

Since the parent__child syntax is being used for list filter, search fields and everywhere else, is there a reason why the list_display has to be sacrosanct?

The real question is, will this be accepted as a patch if contributed on the trunk with test cases et all?

Regards,
Anshuman


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Anshuman Aggarwal  
View profile  
 More options May 17 2011, 7:20 am
From: Anshuman Aggarwal <anshu...@brillgen.com>
Date: Tue, 17 May 2011 04:20:05 -0700 (PDT)
Local: Tues, May 17 2011 7:20 am
Subject: list_display does not allow foreign keys __ syntax
I know this has been discussed (not sure to what depth) but callables
is not the solution. Please bear with me and help me (not force me) to
understand why what I am suggesting is wrong.
I did look in the list and have tried taking this discussion to the
ticket system. http://code.djangoproject.com/ticket/5863

In a nutshell, list_display does not accept foreign key __ syntax
which is being used everywhere (including list_filter as of
1.3)...Just because callables exist as an advanced way of doing custom
items in list does not take away from the argument that __ fk is a
very very common requirement in the list display.

Here are the options currently available:

1. Write functions everywhere (this involves a lot of repetition for
every single foreign field)
2. Write a wrapper like @lukeplant suggested in #5863 (even though it
is far superior solution to individual functions in every model)

These are all HUGE violation of DRY principles on which sensible
programming is based toady.
Most model fields already have a verbose_name, admin_sort_order etc.
defined ...why should it be required to specify in just boilerplate
code all of this all over again? Even in the wrapper function by
@lukeplant, this verbose_name cannot be retrieved from the related
model.

Since the parent__child syntax is being used for list filter, search
fields and everywhere else, is there a reason why the list_display has
to be sacrosanct?

The real question is, will this be accepted as a patch if contributed
on the trunk with test cases et all?

Regards,
Anshuman


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Luke Plant  
View profile  
 More options May 17 2011, 8:18 am
From: Luke Plant <L.Plant...@cantab.net>
Date: Tue, 17 May 2011 13:18:43 +0100
Local: Tues, May 17 2011 8:18 am
Subject: Re: list_display does not allow foreign keys __ syntax
On 17/05/11 12:20, Anshuman Aggarwal wrote:

> Since the parent__child syntax is being used for list filter, search
> fields and everywhere else, is there a reason why the list_display has
> to be sacrosanct?

Please see my response on Trac - I didn't notice you had brought this up
here, so posted there.

Thanks,

Luke

--
Sometimes I wonder if men and women really suit each other. Perhaps
they should live next door and just visit now and then. (Katherine
Hepburn)

Luke Plant || http://lukeplant.me.uk/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Anshuman Aggarwal  
View profile  
 More options May 17 2011, 9:42 am
From: Anshuman Aggarwal <anshu...@brillgen.com>
Date: Tue, 17 May 2011 06:42:38 -0700 (PDT)
Local: Tues, May 17 2011 9:42 am
Subject: Re: list_display does not allow foreign keys __ syntax
I had seen it on trac (since I am following that ticket which has been
marked closed).

I got a mail to not open tickets closed by the core team and to take
up the discussion here. I am putting my reply to your comment here and
keeping the discussion on this thread for resolution.

list_display operates on database fields for all model fields anyways
so its not restricted to python methods/functions.

Comment (by brillgen)

@lukeplant, the basic problem you've correctly raised is that
list_display
allows for callables and hence arbitrary names can be used which are
similar to the syntax for foreign key fields.
However, this problem exists for list_filter also: someone may define
a
field with the name class__field and try to use that in list_filter as
a
foreign key field. (i've just checked that you can indeed name a field
like that)

So we can't really be compensating for dev stupidity at the cost of
functionality as we haven't done in list_filter also.
Since all FKs always start with an alphabet are in the regex format
(.+__.+.*), the __unicode etc cases can be easily and correctly
filtered
as is required to be done in list_filter.

Comment (by lukeplant):

Also, in response to andybak and anonymous, please note:

list_display is fundamentally different from list_filter and
search_fields. The other two are defining operations that must happen
in
the database, whereas list_display is defining an operation that must
happen in Python code, and refers to Python functions/methods, and
**not**
to database fields. The confusion is that a name that of a database
field
is also the name of the corresponding attribute on the Python object,
but
that is where the similarity ends.

So, for instance, you can set `'__unicode__'` in list_display, and it
will
refer to the `__unicode__` method, and not attempt any field lookups,
despite the presence of double underscores. As it happens, this
example
makes it obvious that we cannot sensibly add the feature as proposed -
how
would you refer to the `__unicode__` method on a related object?
`'foreign_key____unicode__'`? How would you parse that? What about
other
methods that people might choose to define, like `__html__`, etc.? The
only way this feature could get in is we are happy with a bunch of
special
cases in the implementation, and a bunch of arbitrary limitations in
functionality.

On May 17, 5:18 pm, Luke Plant <L.Plant...@cantab.net> wrote:


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Luke Plant  
View profile  
 More options May 17 2011, 2:35 pm
From: Luke Plant <L.Plant...@cantab.net>
Date: Tue, 17 May 2011 19:35:24 +0100
Local: Tues, May 17 2011 2:35 pm
Subject: Re: list_display does not allow foreign keys __ syntax
Hi Anshuman,

On 17/05/11 14:42, Anshuman Aggarwal wrote:

OK, back to basics:

ModelAdmin.list_display answers this question:

1) Given a Python object that is a Model instance, what pieces of
information should we display about it in the admin change list?

It answers the question in this way:

We display string representations of A) attributes of the instance,
which can be defined by strings and B) where that does not suffice,
arbitrary callables, which can be defined by strings (referring to
ModelAdmin attributes), or can be actual callables.

However, once you've answered that question, you've got another:

2) Given only a ModelAdmin and/or Model class, how can we define
*titles* (i.e. column headers) for the pieces of information defined by
list_display?

The answer to this is harder, because in general we can't. However, we
can make some pretty good guesses that will cover 95%. These include:

  A) if the piece of information is an attribute that is a field on the
     class, use metadata from the Model field to create the title i.e.
     Field.verbose_name.

     This is a very good guess, because we can be pretty sure that the
     attribute will correspond to the field.

  B) if the attribute is '__str__', use the verbose_name of the Model

Failing this:

  C) Just turn 'some_attribute' into 'Some attribute'

and then:

  D) Allow complete customisation via the 'short_description'
     functionality

Given this is what list_display is about, the suggestion of supporting
'__' like in list_filter is just the wrong thing, because that syntax
has to do with traversing model relationships. Given question (1), which
is the primary question, and the answer we've come up with so far, a
much more appropriate suggestion would be to use dotted path syntax:

  'some_obj.some_field'

Using '__' only makes sense for answering question 2, and only for case
2.A, and this is the reason we end up with silly things like
'foo____str__', which, even if we could get it work robustly, is
extremely ugly.

Then comes the question - should we support the dot syntax? This is
sensible for answering Q1, but not for Q2, because you can't go from
attributes to fields in general. We could, perhaps, support dot syntax
and attempt to do field lookups, but then, firstly, we've got some
implementation problems:

We will either have to

 - duplicate some of the logic from the ORM about traversing fields,
   adding in the attribute lookups.

 - OR put logic into the core ORM that is there to support this
   admin functionality.

(This applies whether we use '__' or '.' syntax).

Both of these are big code smells.

Secondly, we also have a question of what we do this the field
information once we've got it. Suppose we have these models:

class Place(Model):
    short_name = CharField()
    realm = CharField('Country')

class Person(Model):
    full_name = CharField()
    level = CharField() # e.g. BA, PhD

class Department(Model):
    name = CharField()
    manager = ForeignKey(Person)
    location = ForeignKey(Place)

And then:

class DepartmentAdmin(ModelAdmin):
    list_display = ['name',
                    'manager.full_name',
                    'manager.level',
                    'location.short_name',
                    'location.realm'
                   ]

What should the display columns be called? I think I would want:

Name | Manager | Manager level | Location | Country

3 different rules would be required to satisfy all these, and all rules
give really bad results in some cases. Once you add i18n in, some rules
get completely impractical.

The success rate for this kind of guessing can't be much better than
50%, and that is just not worth it - we've already got a 95% solution,
with a method for handling the 5%, so I cannot see that adding a lot of
fragile machinery to get to a 97% solution is going to be worth it.

I could just about see a case for supporting the dotted syntax and not
attempting anything clever to answer Q2, but given the problems
highlighted above, you are very often going to need to use a callable
and 'short_description' to fix things, so I'm still unconvinced it is
worth it. If you have a problem with hundreds of fields that follow a
pattern, I'd suggest using a solution similar to the one I posted on the
ticket.

Best regards,

Luke

--
Sometimes I wonder if men and women really suit each other. Perhaps
they should live next door and just visit now and then. (Katherine
Hepburn)

Luke Plant || http://lukeplant.me.uk/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Anshuman Aggarwal  
View profile  
 More options May 20 2011, 7:08 am
From: Anshuman Aggarwal <anshu...@brillgen.com>
Date: Fri, 20 May 2011 04:08:56 -0700 (PDT)
Local: Fri, May 20 2011 7:08 am
Subject: Re: list_display does not allow foreign keys __ syntax
Hi Luke,
 Thanks for a detailed reply. I have gone over your points carefully
and agree on most except a few key ones.

Based on your response:
 - The question here is really how often is the guesswork right and
for how many different projects/use cases.
I have overseen Django used at more than a few projects and my
observation is that the guesswork logic that you described that is
presently followed for the headers will work very well (~75%) for FK
fields (i.e. take short description/verbose_name/name of field itself)
from the final model.
I realize that this may not be perfect and for advanced cases a
fallback to callables would be required. its anybody guess without
analyzing every single django project out there to say how many people
would benefit.

All in all, without this estimation maybe not worth the effort you
described building the . syntax

I have an alternate suggestion:
Can we develop the accessor callable that you described to be able to
access the short descrition/verbose_name etc and make it available as
a shortcut/decorator in the admin? With that, it won't complicate code
for the main admin, while keeping a quickie around for others??
I have taken your example and developed it a bit (though it still
violates DRY IMHO), but was wondering how the admin can pass in a
parameter with the model description so that it can be traversed to
obtain the field  verbose_name/short_description parameters

def ForeignFieldFunc(field_name, short_description=None,
admin_order_field=None):
    def accessor(obj):
        val = obj
        for part in field_name.split('__'):
            val = getattr(val, part)
        return val
    if short_description:
        accessor.short_description = short_description
    else:
        accessor.__name__ = field_name
    if admin_order_field:
        accessor.admin_order_field = admin_order_field
    else:
        accessor.admin_order_field  = (field_name,)
    return accessor

Regards,
Anshuman

On May 17, 11:35 pm, Luke Plant <L.Plant...@cantab.net> wrote:


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Anshuman Aggarwal  
View profile  
 More options May 20 2011, 7:13 am
From: Anshuman Aggarwal <anshu...@brillgen.com>
Date: Fri, 20 May 2011 04:13:39 -0700 (PDT)
Local: Fri, May 20 2011 7:13 am
Subject: Re: list_display does not allow foreign keys __ syntax
Usage would be: list_display = (ff('model__field__fkfield',
short_description='FK Field') but ideally, the short_description
should be optional with the default logic being used to fetch it out
of the FK.

We can change the __ to a . easily enough without any impact to
anything else.

On May 20, 4:08 pm, Anshuman Aggarwal <anshu...@brillgen.com> wrote:


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Anshuman Aggarwal  
View profile  
 More options May 25 2011, 5:55 am
From: Anshuman Aggarwal <anshu...@brillgen.com>
Date: Wed, 25 May 2011 02:55:36 -0700 (PDT)
Local: Wed, May 25 2011 5:55 am
Subject: Re: list_display does not allow foreign keys __ syntax
Hate to bump a thread, but any thoughts on the last proposal, Luke?

On May 20, 4:13 pm, Anshuman Aggarwal <anshu...@brillgen.com> wrote:


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Luke Plant  
View profile  
 More options Jun 1 2011, 8:58 pm
From: Luke Plant <L.Plant...@cantab.net>
Date: Thu, 02 Jun 2011 01:58:25 +0100
Local: Wed, Jun 1 2011 8:58 pm
Subject: Re: list_display does not allow foreign keys __ syntax
On 25/05/11 10:55, Anshuman Aggarwal wrote:

> Hate to bump a thread, but any thoughts on the last proposal, Luke?

My personal opinion is that the feature you proposed is not going to be
correct enough of the time to warrant the duplication of code that it
would require to work.

Other core devs may have a different opinion, I don't know.

Luke

--
"Trouble: Luck can't last a lifetime, unless you die young."
(despair.com)

Luke Plant || http://lukeplant.me.uk/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Russell Keith-Magee  
View profile  
 More options Jun 1 2011, 10:55 pm
From: Russell Keith-Magee <russ...@keith-magee.com>
Date: Thu, 2 Jun 2011 10:55:46 +0800
Local: Wed, Jun 1 2011 10:55 pm
Subject: Re: list_display does not allow foreign keys __ syntax

On Thu, Jun 2, 2011 at 8:58 AM, Luke Plant <L.Plant...@cantab.net> wrote:
> On 25/05/11 10:55, Anshuman Aggarwal wrote:
>> Hate to bump a thread, but any thoughts on the last proposal, Luke?

> My personal opinion is that the feature you proposed is not going to be
> correct enough of the time to warrant the duplication of code that it
> would require to work.

> Other core devs may have a different opinion, I don't know.

I concur with Luke's analysis. There is a good reason why double
underscore notation isn't appropriate for list_display, and there is
already an elegant way to handle the problem. We don't gain anything
by providing a second approach that will only work some of the time,
crammed into an obscurely named inline function call.

Yours,
Russ Magee %-)


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
End of messages
« Back to Discussions « Newer topic     Older topic »