Re: Ticket #5373

[Lachlan Musicman]

On Fri, Jul 9, 2010 at 00:00, Russell Keith-Magee
<russ...@keith-magee.com> wrote:
> On Thu, Jul 8, 2010 at 3:55 PM, Lachlan Musicman <data...@gmail.com> wrote:
>> Hola,
>>
>> I'm new to this dev thing, but I've done some work on ticket #5373

Another couple of hours of pdb'ing my way into this problem...

> So - the next step is to make the tests more robust; you'll either
> prove that this isn't a problem, or you'll find the edge case that
> needs to be fixed.

> As for the tests themselves; a general rule for testing is to test as
> close to the source of the problem as possible. While I'm sure you
> *can* observe this problem in the admin, the fact that the fix doesn't
> involve any admin-specific code leads me to suspect that a better test
> would be at a lower level -- in this case, testing that foreign key
> fields return an appropriate verbose_name, and/or that ModelForms pick
> up a translated verbose name.

If you go to the ticket, you will see that I've added a new patch for
django/db/models/fields/related.py

This shows that the original change made in ticket 8011 (revision
8132) was not refactored properly.

I have refactored 8011 so that it now does the same thing cleaner.

Note that this does not solve the 5373 problem, merely proves that
there is a problem.

cheers
L.

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

Re: Ticket #5373

[Lachlan Musicman]

On Sun, Jul 11, 2010 at 17:31, Russell Keith-Magee
 wrote:
>>  wrote:
>> I'm not quite sure I understand. Does that mean we can have
>> conflicting verbose_names? My mental unrolling of your advice brings
>> me to:
>
> The two verbose names are describing different things.
>
> "Novel" is the verbose name for a book object. The admin page for a
> list of "books" will be titled "Novel", and say "select a Novel", etc.
>
> "Paperback" is the verbose name for the relationship held between
> Authors and Books.

ok, got it. Cheers

> The only reason to get verbose_name and verbose_name_plural involved
> is when the automated scheme for class name de-munging won't work. The
> obvious use case for this is plurals: your list of Walrus objects
> should say "You have 3 Walruses", not "You have 3 Walruss".

In that case, how would I go about localizing the class name without
verbose_name? Surely that is a second reason to use verbose_name?

> The edge case that I'm referring to is the case where a verbose name
> isn't specified on the field, and you get one inherited from the
> object you're related to.

The problem I'm referring to is that the de-munged class name of a FK
on the admin page of the class containing that FK isn't acknowledging
the localisation?

I've added another working patch to the ticket, but no test for it
yet, and I'm sure I'm stil doing something wrong.

cheers
L.

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

Re: Ticket #5373

[Russell Keith-Magee]

On Sun, Jul 11, 2010 at 12:53 PM, Lachlan Musicman <data...@gmail.com> wrote:
> On Fri, Jul 9, 2010 at 00:00, Russell Keith-Magee
> <russ...@keith-magee.com> wrote:
>> On Thu, Jul 8, 2010 at 3:55 PM, Lachlan Musicman <data...@gmail.com> wrote:
>>> Hola,
>>>
>>> I'm new to this dev thing, but I've done some work on ticket #5373
>
>> However, I'm also concerned about unexpected consequences for the
>> second patch. The obvious use case that you're breaking is if your
>> ForeignKey manually defines a verbose_name.
>
> I'm not quite sure I understand. Does that mean we can have
> conflicting verbose_names? My mental unrolling of your advice brings
> me to:
>
> eg 1:
> class Book(models.Model):
>    name = models.CharField(max_length=50)
>
>    class Meta:
>        verbose_name = _(u'Novel')
>        verbose_name_plural = _(u'Novels')
>
> class Author(models.Model):
>    name = models.CharField(max_length=50)
>    books = models.ManyToManyField(Book, verbose_name=_(u'Paperback'))
>
> In this (ugly, but possible?) case, we have conflicting verbose names.
> Which should take precedence? Your note would indicate that Paperback
> should?

The two verbose names are describing different things.

"Novel" is the verbose name for a book object. The admin page for a
list of "books" will be titled "Novel", and say "select a Novel", etc.

"Paperback" is the verbose name for the relationship held between
Authors and Books.

Of course, your example here is a little pathological; ordinarily, if
you want to have a list of novels, you'd call the object "class
Novel(Model)", and if you want an author to have a list of paperbacks,
you'd call the relationship "paperback = models.ManyToMany(Book...)".
The only reason to get verbose_name and verbose_name_plural involved
is when the automated scheme for class name de-munging won't work. The
obvious use case for this is plurals: your list of Walrus objects
should say "You have 3 Walruses", not "You have 3 Walruss".

The edge case that I'm referring to is the case where a verbose name
isn't specified on the field, and you get one inherited from the
object you're related to.

Yours,
Russ Magee %-)

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

Re: Ticket #5373

[Lachlan Musicman]

On Fri, Jul 9, 2010 at 00:00, Russell Keith-Magee
<russ...@keith-magee.com> wrote:
> On Thu, Jul 8, 2010 at 3:55 PM, Lachlan Musicman <data...@gmail.com> wrote:
>> Hola,
>>
>> I'm new to this dev thing, but I've done some work on ticket #5373
>
> Thanks for pitching in! Hopefully I'll be able to give you enough
> feedback to progress this issue without scaring you off :-)

That would be great, cheers

> I agree that the first patch (.2.patch) isn't the right approach --
> isinstance checks are generally an indication that you're doing
> something wrong (or at least that you could be doing it better).

Yeah, I knew that, just didn't have the eloquence

> However, I'm also concerned about unexpected consequences for the
> second patch. The obvious use case that you're breaking is if your
> ForeignKey manually defines a verbose_name.

I'm not quite sure I understand. Does that mean we can have
conflicting verbose_names? My mental unrolling of your advice brings
me to:

eg 1:
class Book(models.Model):
name = models.CharField(max_length=50)

class Meta:
verbose_name = _(u'Novel')
verbose_name_plural = _(u'Novels')

class Author(models.Model):
name = models.CharField(max_length=50)
books = models.ManyToManyField(Book, verbose_name=_(u'Paperback'))

In this (ugly, but possible?) case, we have conflicting verbose names.
Which should take precedence? Your note would indicate that Paperback
should?


> So - the next step is to make the tests more robust; you'll either
> prove that this isn't a problem, or you'll find the edge case that
> needs to be fixed.

It's definitely a problem - I've started this because it's one I experience :)

> would be at a lower level -- in this case, testing that foreign key
> fields return an appropriate verbose_name, and/or that ModelForms pick
> up a translated verbose name.

Got it - am working on it now.

cheers
L.

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

Re: Ticket #5373

[Russell Keith-Magee]

On Thu, Jul 8, 2010 at 3:55 PM, Lachlan Musicman <data...@gmail.com> wrote:
> Hola,
>
> I'm new to this dev thing, but I've done some work on ticket #5373
>
> http://code.djangoproject.com/ticket/5373  Field label for a
> ForeignKey not translated

Thanks for pitching in! Hopefully I'll be able to give you enough
feedback to progress this issue without scaring you off :-)

> There are two different patches fixing the same problem - I've no idea
> which is considered better. I don't like the late import on the first
> patch (5373.2.patch), and the second may have unexpected consequences
> (5373_related.py.diff ).
>
> I've also added a patch for tests/regressiontests/admin_inlines that
> confirms that either patch works.

I agree that the first patch (.2.patch) isn't the right approach --
isinstance checks are generally an indication that you're doing
something wrong (or at least that you could be doing it better).

However, I'm also concerned about unexpected consequences for the
second patch. The obvious use case that you're breaking is if your
ForeignKey manually defines a verbose_name.

So - the next step is to make the tests more robust; you'll either
prove that this isn't a problem, or you'll find the edge case that
needs to be fixed.

As for the tests themselves; a general rule for testing is to test as
close to the source of the problem as possible. While I'm sure you
*can* observe this problem in the admin, the fact that the fix doesn't
involve any admin-specific code leads me to suspect that a better test
would be at a lower level -- in this case, testing that foreign key
fields return an appropriate verbose_name, and/or that ModelForms pick
up a translated verbose name.

Yours,
Russ Magee %-)

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

Ticket #5373

[Lachlan Musicman]

Hola,

I'm new to this dev thing, but I've done some work on ticket #5373

http://code.djangoproject.com/ticket/5373  Field label for a
ForeignKey not translated

There are two different patches fixing the same problem - I've no idea
which is considered better. I don't like the late import on the first
patch (5373.2.patch), and the second may have unexpected consequences
(5373_related.py.diff ).

I've also added a patch for tests/regressiontests/admin_inlines that
confirms that either patch works.

I'm not sure what to do next?

cheers
L.

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