Re: Proposal: make Model __unicode__() default to self.name

2017-12-11 Thread Alex Corcoles 
Sorry to resurrect this, but I commented on the bug tracker ( 
https://code.djangoproject.com/ticket/28839#comment:4 ) and was told to 
take the discussion here.

I want to point out that __str__ is used for "GUI" purposes (such as 
dropdown texts in ModelForm/ModelAdmin) and putting a default __str__ with 
a PK doesn't seem to make sense. I think __str__ is a more 
user/human-friendly thing, which I don't think can be generated 
automatically.

On the other hand, it does make sense to include the PK in __repr__, so I 
would suggest backtracking on this change and making an equivalent change 
in __repr__.

On Saturday, April 8, 2017 at 8:41:56 PM UTC+2, Collin Anderson wrote:
>
> I just made a pull request.
> https://github.com/django/django/pull/8336
>
> (1) is my first choice, pk=1 is my second choice. I'd be fine with either.
>
> On Sat, Apr 8, 2017 at 12:08 PM, Marco Silva  > wrote:
>
>> just saw that __repr__ is now under discusion here
>> https://groups.google.com/forum/#!topic/django-developers/UNKFMg6DO5s
>>
>>
>> sábado, 8 de Abril de 2017 às 17:06:05 UTC+1, Marco Silva escreveu:
>>>
>>> I have no idea what is the best way, just say that comment. this is the 
>>> original PR
>>>
>>> https://github.com/django/django/commit/d2a26c1a90e83dabdf3d67ceec4d2a70fb86
>>>
>>> I think you should submit the PR to change the __str__ method, and maybe 
>>> open a new discussion regarding __repr__
>>>
>>> sexta-feira, 7 de Abril de 2017 às 15:34:32 UTC+1, Kapil Garg escreveu:

 The opened ticket is about Model.__str__ method. Should i open a new 
 ticket for this change ?
 As i see in code, self.__class__ is used in a lot of places but will it 
 effect optimization if we change lookups from self.__class__ to self.cls

 Because the methods where class is being used frequently, already store 
 it in local variable and then make references to local variable. 

 So should it really be changed ?

 On Fri, Apr 7, 2017, 6:52 PM Marco Silva  wrote:

> I noticed this on the init
>
> def __init__(self, *args, **kwargs):
>  # Alias some things as locals to avoid repeat global lookups
>  cls = self.__class__
>
> maybe you should change it to self.cls??
> Try to submit a PR to the open ticket.
>
> segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg 
> escreveu:
>>
>> So does this patch seem fine ? 
>>
>> diff --git a/django/db/models/base.py b/django/db/models/base.py
>> index 3c1cb9a..f58e12b 100644
>> --- a/django/db/models/base.py
>> +++ b/django/db/models/base.py
>> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>>  return '<%s: %s>' % (self.__class__.__name__, u)
>>  
>>  def __str__(self):
>> -return '%s object' % self.__class__.__name__
>> +return '%s object pk=%s' % (self.__class__.__name__, 
>> self._get_pk_val())
>>  
>>  def __eq__(self, other):
>>  if not isinstance(other, Model):
>>
>>
>>
>>
>> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>>>
>>> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may 
>>> default to generating a pk before save, so it might just be best to 
>>> show 
>>> the actual current pk value
>>>
>>> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  
>>> wrote:
>>>
 So what should the final __str__ show: Should it be 'ClassName 
 object pk=Something' and if pk is None then should it be 'ClassName 
 object 
 (unsaved)' or 'ClassName object pk=None' ?

 On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:
>
> Makes sense to me. Maybe still keep the "Transaction object" part, 
> and use None if no pk.
>
> On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  
> wrote:
>
>> Ticket 27953  is 
>> regarding this proposal and the suggestion is about adding "pk" in 
>> Model 
>> string representation if it exists. 
>>
>> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson 
>> wrote:
>>>
>>> Hi All,
>>>
>>> Have you ever quickly set up a model, ran syncdb, and added a 
>>> few sample objects in the admin to only see a bunch of "MyModel 
>>> object"s in 
>>> the changelist? I always forget to add a __unicode__()/__str__() 
>>> method on 
>>> my models.
>>>
>>> I ran "git grep -1 __unicode__" on some of my django projects 
>>> and noticed a lot of repeated code. In fact, it seems that in about 
>>> a 
>>> _third_ of all my cases, I'm just returning self.name, or 
>>> returning self.name would have been a good default. I looked at 
>>> a few 3rd party apps f

Re: Proposal: make Model __unicode__() default to self.name

2017-12-12 Thread Alex Corcoles 
Hi Collin,

On Tuesday, December 12, 2017 at 3:48:38 PM UTC+1, Collin Anderson wrote:

> Yes, having a good __str__ is important, and thats why i think defaulting 
> it to the pk is good, because it's better than nothing. Yes, you'll still 
> want to override __str__ with something better (that likely doesn't include 
> the PK).
>

I slightly disagree on this specific point, but I don't think my squabble 
is worth wasting anyone's time :)

So if you're overriding __str__ anyway, why not also override __repr__ to 
> include the PK?
>
> Or what are you actually proposing, something like this?
> https://github.com/django/django/compare/master...collinanderson:patch-12
>
 
Yeah, I just want something like your PR. I want to override __str__ most 
of the time, so having it include the PK doesn't do much for me- neither 
good or bad, but having the default __repr__ include the PK saves me from 
overriding it most of the time.

I could nitpick on my ideal implementation of both methods, but I think it 
comes down to personal taste so I just wanted to propose having PK in the 
default __repr__ implementation as I think that will benefit people. The 
rest doesn't matter that much to me.

Thank you,

Álex

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b0618a9b-fc58-4057-a06f-16bd884ebdc3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.