Re: Ticket for Docs improvement Was: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Łukasz Rekucki
On 5 April 2012 19:45, Alex Ogier  wrote:
>
>> >> class User(models.Model):
>> >>     MALE = 0
>> >>     FEMALE = 1
>> >>     GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
>> >>     gender = models.IntegerField(choices=GENDERS)
>> >>
>> >>     def greet(self):
>> >>         return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]
>> >>
>>
>> I' sure you meant:
>>
>> def greet(self):
>>    return {self.MALE: 'Hi, boy', self.FEMALE: 'Hi, girl.'}[self.gender]
>>
>> Unless you defined MALE/FEMALE as globals too :) Otherwise you'll get
>> a NameError.
>>
>> --
>> Łukasz Rekucki
>
> As attributes of the class object I'm pretty sure they are in scope. No
> NameErrors there.
>

That's not how it works. Code that executes when creating a new class
does not define a lexical scope. There is no such thing as "class
scope". Try it yourself:

http://ideone.com/xbr0q

-- 
Łukasz Rekucki

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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 for Docs improvement Was: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Alex Ogier
> >> class User(models.Model):
> >> MALE = 0
> >> FEMALE = 1
> >> GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
> >> gender = models.IntegerField(choices=GENDERS)
> >>
> >> def greet(self):
> >> return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]
> >>
>
> I' sure you meant:
>
> def greet(self):
>return {self.MALE: 'Hi, boy', self.FEMALE: 'Hi, girl.'}[self.gender]
>
> Unless you defined MALE/FEMALE as globals too :) Otherwise you'll get
> a NameError.
>
> --
> Łukasz Rekucki

As attributes of the class object I'm pretty sure they are in scope. No
NameErrors there.

Best,
Alex Ogier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Tom Evans
2012/4/4 Łukasz Langa :
> Wiadomość napisana przez Tom Evans w dniu 4 kwi 2012, o godz. 18:40:
>
>> The class definition grates. When we say things like:
>>
>>  class Gender(Choices):
>>    male = Choice("male")
>>
>> That says to me that Gender.male is mutable. Ick.
>
> Thanks for your feedback. Do model and form field definitions say that as 
> well? It's true that I could have block users from mutating the value but 
> "we're all consenting adults here", right?

Yes, they do. If I have a model with a field named 'address', I expect
that different instances of that model may have different values for
that attribute.

If on the other hand, I have a class with an attribute 'MALE', then I
expect that value to be immutable.

>
>> There is no easy way by inspecting the code to see what choice a value
>> of 7 equates to any-more.
>
> True, the same goes for spotting errors with manual numbering when there are 
> many values.

That makes no sense. If you want to see what value a choice has with
the proposed system, you have to work out precisely what ids will be
automagically assigned to a particular choice.

If you want to spot errors with the current system, you simply look
for choices with the same number. You don't have to work out what
number a choice is, it is explicitly there.

>
>> Do the choices start from 0 or do they start
>> from 1, as inferred by the examples (Why? What is wrong with 0?).
>> Repetition isn't good, but ambiguity is worse.
>
> It's a matter of aesthetics and as such it's totally subjective. I made the 
> values start from 1 so that the first Choice.Group can have value 0 and not 
> -1 which looks ugly.
>
>> This grouping system just seems destined for data loss/confusion. If I
>> want to split a group in two, the enums in the new group change
>> values! That is not a good approach, and was not necessary with the
>> old system.
>
> I can't see how they have to.

So, If I take your examples for groups, and annotate it with the
automagic assigned id:

> class License(Choices):
> COPYLEFT = Choices.Group(0)
> gpl_any = Choice("GPL, any")# id: 1
> gpl2 = Choice("GPL 2")  # id: 2
> gpl3 = Choice("GPL 3")  # id: 2
> lgpl = Choice("LGPL")   # id: 3
> agpl = Choice("Affero GPL") # id: 4
>
> PUBLIC_DOMAIN = Choices.Group(100)
> bsd = Choice("BSD") # id: 101
> public_domain = Choice("Public domain") # id: 102
>
> OSS = Choices.Group(200)
> apache2 = Choice("Apache 2")# id: 201
> mozilla = Choice("MPL") # id: 202

and now I decide that I want BSD licenses as a separate group to
public domain licenses, as they are not the same thing.

> class License(Choices):
> COPYLEFT = Choices.Group(0)
> gpl_any = Choice("GPL, any")# id: 1
> gpl2 = Choice("GPL 2")  # id: 2
> gpl3 = Choice("GPL 3")  # id: 2
> lgpl = Choice("LGPL")   # id: 3
> agpl = Choice("Affero GPL") # id: 4
>
> PUBLIC_DOMAIN = Choices.Group(100)
> public_domain = Choice("Public domain") # id: 101
>
> BSD = Choices.Group(150)
> bsd = Choice("BSD") # id: 151
>
> OSS = Choices.Group(200)
> apache2 = Choice("Apache 2")# id: 201
> mozilla = Choice("MPL") # id: 202

public_domain has changed from 102 => 101
bsd has changed from 101 => 151

These ids only change because ids in this system are handed out
automagically in order of appearance in the class, and grouping is
handled by order of appearance in the class.

>> If I had a vote, I'd be strongly -1 on
>> any proposal with this sort of grouping, it seems dangerous and wrong.
>
> Can you elaborate on what is dangerous about them?

So as above. Your counter will be "well, in those cases you need to
explicitly set an id on those choices". This relies on the person
making the changes realizing that there is magic happening, and take
that into account. That is dangerous compared to the current system,
where I would trust even a novice developer to change and move around
options.

This is because the current system is explicitly clear in what is happening.

>
>> Finally, the proposal seems to concentrate solely on choices as an
>> enum. The proposal completely ignores that choices can be almost any
>> kind of value, eg:
>>
>> MALE="m"
>> FEMALE="f"
>> UNKNOWN="?"
>> GENDER_CHOICES = (
>>    (MALE, "Male"),
>>    (FEMALE, "Female"),
>>    (UNKNOWN, "Unknown"),
>>  )
>>
>> this would be an entirely appropriate choices for a CharField.
>
> Using code in my proposal:
>
 class Gender(Choices):
> ...     m = Choice("male")
> ...     f = Choice("female")
> ...     n = Choice("not specified")
> ...
 Gender(item=lambda c: (c.name, c.desc))
> [(u'm', u'male'), (u'f', u'female'), (u'n', u'not specified')]
>

So instead of MALE and FEMALE, Gender.m? No 

Re: Ticket for Docs improvement Was: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Thomas Guettler



Am 05.04.2012 10:03, schrieb Łukasz Rekucki:

On 5 April 2012 09:45, Thomas Guettler  wrote:

Hi,

I created a ticket, incl. patch

https://code.djangoproject.com/ticket/18062




While the example itself is useful, I don't really think Django's
documentation should state obvious facts about Python, especially
false ones.

...

Unless you defined MALE/FEMALE as globals too :) Otherwise you'll get
a NameError.


patch is fixed now. Unfortunately I can't modify the description in trac.

  Thomas

--
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Łukasz Langa
Wiadomość napisana przez Stephen Burrows w dniu 5 kwi 2012, o godz. 07:41:

> I generally like this idea, except for the implicit integer ids based
> on declaration order.

Thanks for your input. I will add the ability to explicitly set .id values for 
each Choice in the next release.

> As people have said, it seems like just asking
> for data corruption.

Fair enough, although that does not match my experience.

> I'd much rather see implicit character ids based
> on the attribute name of the choice, analogous to django fields.


>>> class Gender(Choices):
... m = Choice("male")
... f = Choice("female")
... n = Choice("not specified")
... 
>>> Gender(item=lambda c: (c.name, c.desc))
[(u'm', u'male'), (u'f', u'female'), (u'n', u'not specified')]


> Will you be making this project available as a third-party app?

pip install dj.choices

-- 
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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 for Docs improvement Was: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Łukasz Langa
Wiadomość napisana przez Łukasz Rekucki w dniu 5 kwi 2012, o godz. 10:03:

> I' sure you meant:
> 
> def greet(self):
>return {self.MALE: 'Hi, boy', self.FEMALE: 'Hi, girl.'}[self.gender]
> 
> Unless you defined MALE/FEMALE as globals too :) Otherwise you'll get
> a NameError.


Also, a minor diversity note: in real life gender choice is not binary.

-- 
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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 for Docs improvement Was: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Łukasz Rekucki
On 5 April 2012 09:45, Thomas Guettler  wrote:
> Hi,
>
> I created a ticket, incl. patch
>
> https://code.djangoproject.com/ticket/18062
>
>

While the example itself is useful, I don't really think Django's
documentation should state obvious facts about Python, especially
false ones.

>
> Am 04.04.2012 18:41, schrieb Adrian Holovaty:
>>
>>
>> I don't see the immediate need for Yet Another Sub-framework, as
>> described in this proposal. This is what I normally do, and it works
>> fine:
>>
>> class User(models.Model):
>>     MALE = 0
>>     FEMALE = 1
>>     GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
>>     gender = models.IntegerField(choices=GENDERS)
>>
>>     def greet(self):
>>         return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]
>>

I' sure you meant:

def greet(self):
return {self.MALE: 'Hi, boy', self.FEMALE: 'Hi, girl.'}[self.gender]

Unless you defined MALE/FEMALE as globals too :) Otherwise you'll get
a NameError.

-- 
Łukasz Rekucki

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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 for Docs improvement Was: Proposal: upgrading the choices machinery for Django

2012-04-05 Thread Thomas Guettler

Hi,

I created a ticket, incl. patch

https://code.djangoproject.com/ticket/18062



Am 04.04.2012 18:41, schrieb Adrian Holovaty:

2012/4/3 Łukasz Langa:

Explicit choice values::

  GENDER_MALE = 0
  GENDER_FEMALE = 1
  GENDER_NOT_SPECIFIED = 2

  GENDER_CHOICES = (
  (GENDER_MALE, _('male')),
  (GENDER_FEMALE, _('female')),
  (GENDER_NOT_SPECIFIED, _('not specified')),
  )

  class User(models.Model):
  gender = models.IntegerField(choices=GENDER_CHOICES,
  default=GENDER_NOT_SPECIFIED)

  def greet(self):
  if self.gender == GENDER_MALE:
  return 'Hi, boy.'
  elif self.gender == GENDER_NOT_SPECIFIED:
  return 'Hello, girl.'
  else: return 'Hey there, user!'

This is a saner way but starts getting overly verbose and redundant. You can
improve encapsulation by moving the choices into the ``User`` class but that on
the other hand beats reusability.


I don't see the immediate need for Yet Another Sub-framework, as
described in this proposal. This is what I normally do, and it works
fine:

class User(models.Model):
 MALE = 0
 FEMALE = 1
 GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
 gender = models.IntegerField(choices=GENDERS)

 def greet(self):
 return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]

If people aren't understanding that, we should improve our documentation.

Also, the fact that we *don't* have a sub-framework for this lets
people do whatever they want -- including using the dj.choices module.
So I am -1 on including this in Django proper.

Adrian



--
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Stephen Burrows
I generally like this idea, except for the implicit integer ids based
on declaration order. As people have said, it seems like just asking
for data corruption. I'd much rather see implicit character ids based
on the attribute name of the choice, analogous to django fields.

Will you be making this project available as a third-party app?

--Stephen

On Apr 4, 5:41 pm, Alex Ogier  wrote:
> On Wed, Apr 4, 2012 at 5:18 PM, Łukasz Langa  wrote:
> > Wiadomość napisana przez Daniel Greenfeld w dniu 4 kwi 2012, o godz. 22:48:
>
> > > On two occasions now I've had to do serious debugging on implementations
> > done by other people who used a technique not dissimilar to what Łukasz
> > proposes. In both cases, while the inheritance and better control
> > structures were nice, the issue was that if you give people enough rope to
> > hang themselves, they will.
>
> > Can you elaborate on that a bit? Even if the proposal is rejected in the
> > end, I might use your experience to at least make the rope less deadly.
>
> I imagine a big one is that without an explicit database representation for
> every item, it is easy to get out of sync with the real data you have. And
> when that happens, there isn't necessarily a good way of retrieving the
> meaning of old data: if some data in your database says that you are using
> license '302' but that index was implicitly auto-generated by the
> particular ordering of Choice() instantiations, then when the ordering
> changes you can lose track (and indexing based on an implicit ordering of
> class attributes definitely seems shaky to me).
>
> When I use enumerated fields, I generally make them VARCHAR(10) or so, and
> then use plain English, potentially abbreviated, for a database
> representation. That makes for reasonable code, "if self.license ==
> 'gpl_any':" looks very readable to me, and doesn't restrict you from
> altering display values for internationalization.
>
> It seems to me like this is really a documentation problem, where
> distilling the wisdom of developers like Adrian into a little best
> practices paragraph in the choices argument reference would go very far in
> making the awkwardness go away.
>
> Best,
> Alex Ogier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Alex Ogier
On Wed, Apr 4, 2012 at 5:18 PM, Łukasz Langa  wrote:

> Wiadomość napisana przez Daniel Greenfeld w dniu 4 kwi 2012, o godz. 22:48:
>
> > On two occasions now I've had to do serious debugging on implementations
> done by other people who used a technique not dissimilar to what Łukasz
> proposes. In both cases, while the inheritance and better control
> structures were nice, the issue was that if you give people enough rope to
> hang themselves, they will.
>
> Can you elaborate on that a bit? Even if the proposal is rejected in the
> end, I might use your experience to at least make the rope less deadly.


I imagine a big one is that without an explicit database representation for
every item, it is easy to get out of sync with the real data you have. And
when that happens, there isn't necessarily a good way of retrieving the
meaning of old data: if some data in your database says that you are using
license '302' but that index was implicitly auto-generated by the
particular ordering of Choice() instantiations, then when the ordering
changes you can lose track (and indexing based on an implicit ordering of
class attributes definitely seems shaky to me).

When I use enumerated fields, I generally make them VARCHAR(10) or so, and
then use plain English, potentially abbreviated, for a database
representation. That makes for reasonable code, "if self.license ==
'gpl_any':" looks very readable to me, and doesn't restrict you from
altering display values for internationalization.

It seems to me like this is really a documentation problem, where
distilling the wisdom of developers like Adrian into a little best
practices paragraph in the choices argument reference would go very far in
making the awkwardness go away.

Best,
Alex Ogier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Łukasz Langa
Wiadomość napisana przez Daniel Greenfeld w dniu 4 kwi 2012, o godz. 22:48:

> +1 to what Adrian says.

Thanks for your input! :)

> On two occasions now I've had to do serious debugging on implementations done 
> by other people who used a technique not dissimilar to what Łukasz proposes. 
> In both cases, while the inheritance and better control structures were nice, 
> the issue was that if you give people enough rope to hang themselves, they 
> will.

Can you elaborate on that a bit? Even if the proposal is rejected in the end, I 
might use your experience to at least make the rope less deadly.

-- 
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Daniel Greenfeld
On Wednesday, April 4, 2012 9:41:23 AM UTC-7, Adrian Holovaty wrote:

> 2012/4/3 Łukasz Langa :
> > Explicit choice values::
> >
> >  GENDER_MALE = 0
> >  GENDER_FEMALE = 1
> >  GENDER_NOT_SPECIFIED = 2
> >
> >  GENDER_CHOICES = (
> >  (GENDER_MALE, _('male')),
> >  (GENDER_FEMALE, _('female')),
> >  (GENDER_NOT_SPECIFIED, _('not specified')),
> >  )
> >
> >  class User(models.Model):
> >  gender = models.IntegerField(choices=GENDER_CHOICES,
> >  default=GENDER_NOT_SPECIFIED)
> >
> >  def greet(self):
> >  if self.gender == GENDER_MALE:
> >  return 'Hi, boy.'
> >  elif self.gender == GENDER_NOT_SPECIFIED:
> >  return 'Hello, girl.'
> >  else: return 'Hey there, user!'
> >
> > This is a saner way but starts getting overly verbose and redundant. You 
> can
> > improve encapsulation by moving the choices into the ``User`` class but 
> that on
> > the other hand beats reusability.
>
> I don't see the immediate need for Yet Another Sub-framework, as
> described in this proposal. This is what I normally do, and it works
> fine:
>
> class User(models.Model):
> MALE = 0
> FEMALE = 1
> GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
> gender = models.IntegerField(choices=GENDERS)
>
> def greet(self):
> return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]
>
> If people aren't understanding that, we should improve our documentation.
>
> Also, the fact that we *don't* have a sub-framework for this lets
> people do whatever they want -- including using the dj.choices module.
> So I am -1 on including this in Django proper.
>
> Adrian
>


+1 to what Adrian says. In fact...

On two occasions now I've had to do serious debugging on implementations 
done by other people who used a technique not dissimilar to what Łukasz 
proposes. In both cases, while the inheritance and better control 
structures were nice, the issue was that if you give people enough rope to 
hang themselves, they will.

The current system works very well, especially when you follow Adrian's 
pattern. Lukasz' system works well too. Therefore...

I submit that we keep Django core for choices as is and let people who want 
to use another system implement 3rd party packages. 
 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/F20PJZowsZ4J.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Łukasz Langa
Wiadomość napisana przez Adrian Holovaty w dniu 4 kwi 2012, o godz. 18:41:

> Also, the fact that we *don't* have a sub-framework for this lets
> people do whatever they want -- including using the dj.choices module.
> So I am -1 on including this in Django proper.

Thank you for your feedback. As you're one of the BFDLs for the project, that 
basically closes the proposal as rejected, am I right?

-- 
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Łukasz Langa
Wiadomość napisana przez Tom Evans w dniu 4 kwi 2012, o godz. 18:40:

> The class definition grates. When we say things like:
> 
>  class Gender(Choices):
>male = Choice("male")
> 
> That says to me that Gender.male is mutable. Ick.

Thanks for your feedback. Do model and form field definitions say that as well? 
It's true that I could have block users from mutating the value but "we're all 
consenting adults here", right?

> There is no easy way by inspecting the code to see what choice a value
> of 7 equates to any-more.

True, the same goes for spotting errors with manual numbering when there are 
many values.

> Do the choices start from 0 or do they start
> from 1, as inferred by the examples (Why? What is wrong with 0?).
> Repetition isn't good, but ambiguity is worse.

It's a matter of aesthetics and as such it's totally subjective. I made the 
values start from 1 so that the first Choice.Group can have value 0 and not -1 
which looks ugly.

> This grouping system just seems destined for data loss/confusion. If I
> want to split a group in two, the enums in the new group change
> values! That is not a good approach, and was not necessary with the
> old system.

I can't see how they have to.

> If I had a vote, I'd be strongly -1 on
> any proposal with this sort of grouping, it seems dangerous and wrong.

Can you elaborate on what is dangerous about them?

> Finally, the proposal seems to concentrate solely on choices as an
> enum. The proposal completely ignores that choices can be almost any
> kind of value, eg:
> 
> MALE="m"
> FEMALE="f"
> UNKNOWN="?"
> GENDER_CHOICES = (
>(MALE, "Male"),
>(FEMALE, "Female"),
>(UNKNOWN, "Unknown"),
>  )
> 
> this would be an entirely appropriate choices for a CharField.

Using code in my proposal:

>>> class Gender(Choices):
... m = Choice("male")
... f = Choice("female")
... n = Choice("not specified")
... 
>>> Gender(item=lambda c: (c.name, c.desc))
[(u'm', u'male'), (u'f', u'female'), (u'n', u'not specified')]

-- 
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Sean Brant
I agree we don't really gain anything from including this in core.
Django model utils[1] has a pretty solid implementation of a choice
abstraction.

[1] https://github.com/carljm/django-model-utils

On Wed, Apr 4, 2012 at 11:41 AM, Adrian Holovaty  wrote:
> 2012/4/3 Łukasz Langa :
>> Explicit choice values::
>>
>>  GENDER_MALE = 0
>>  GENDER_FEMALE = 1
>>  GENDER_NOT_SPECIFIED = 2
>>
>>  GENDER_CHOICES = (
>>      (GENDER_MALE, _('male')),
>>      (GENDER_FEMALE, _('female')),
>>      (GENDER_NOT_SPECIFIED, _('not specified')),
>>  )
>>
>>  class User(models.Model):
>>      gender = models.IntegerField(choices=GENDER_CHOICES,
>>              default=GENDER_NOT_SPECIFIED)
>>
>>      def greet(self):
>>          if self.gender == GENDER_MALE:
>>              return 'Hi, boy.'
>>          elif self.gender == GENDER_NOT_SPECIFIED:
>>              return 'Hello, girl.'
>>          else: return 'Hey there, user!'
>>
>> This is a saner way but starts getting overly verbose and redundant. You can
>> improve encapsulation by moving the choices into the ``User`` class but that 
>> on
>> the other hand beats reusability.
>
> I don't see the immediate need for Yet Another Sub-framework, as
> described in this proposal. This is what I normally do, and it works
> fine:
>
> class User(models.Model):
>    MALE = 0
>    FEMALE = 1
>    GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
>    gender = models.IntegerField(choices=GENDERS)
>
>    def greet(self):
>        return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]
>
> If people aren't understanding that, we should improve our documentation.
>
> Also, the fact that we *don't* have a sub-framework for this lets
> people do whatever they want -- including using the dj.choices module.
> So I am -1 on including this in Django proper.
>
> Adrian
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
> To post to this group, send email to django-developers@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.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Adrian Holovaty
2012/4/3 Łukasz Langa :
> Explicit choice values::
>
>  GENDER_MALE = 0
>  GENDER_FEMALE = 1
>  GENDER_NOT_SPECIFIED = 2
>
>  GENDER_CHOICES = (
>      (GENDER_MALE, _('male')),
>      (GENDER_FEMALE, _('female')),
>      (GENDER_NOT_SPECIFIED, _('not specified')),
>  )
>
>  class User(models.Model):
>      gender = models.IntegerField(choices=GENDER_CHOICES,
>              default=GENDER_NOT_SPECIFIED)
>
>      def greet(self):
>          if self.gender == GENDER_MALE:
>              return 'Hi, boy.'
>          elif self.gender == GENDER_NOT_SPECIFIED:
>              return 'Hello, girl.'
>          else: return 'Hey there, user!'
>
> This is a saner way but starts getting overly verbose and redundant. You can
> improve encapsulation by moving the choices into the ``User`` class but that 
> on
> the other hand beats reusability.

I don't see the immediate need for Yet Another Sub-framework, as
described in this proposal. This is what I normally do, and it works
fine:

class User(models.Model):
MALE = 0
FEMALE = 1
GENDERS = [(MALE, 'Male'), (FEMALE, 'Female')]
gender = models.IntegerField(choices=GENDERS)

def greet(self):
return {MALE: 'Hi, boy', FEMALE: 'Hi, girl.'}[self.gender]

If people aren't understanding that, we should improve our documentation.

Also, the fact that we *don't* have a sub-framework for this lets
people do whatever they want -- including using the dj.choices module.
So I am -1 on including this in Django proper.

Adrian

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Tom Evans
2012/4/3 Łukasz Langa :
> A nice HTML rendering of this proposal is available at:
> http://lukasz.langa.pl/2/upgrading-choices-machinery-django/

Disclaimer: all my code currently looks like 'Way 3'.

In general, I like it. There are a few things I don't like in it:

The class definition grates. When we say things like:

  class Gender(Choices):
male = Choice("male")

That says to me that Gender.male is mutable. Ick.


There is no easy way by inspecting the code to see what choice a value
of 7 equates to any-more. Do the choices start from 0 or do they start
from 1, as inferred by the examples (Why? What is wrong with 0?).
Repetition isn't good, but ambiguity is worse.


I don't like the grouping at all. Grouping under the old system was
clean and simple, it didn't require magic arguments, groups didn't
require that all options are contiguous in a single range.

This grouping system just seems destined for data loss/confusion. If I
want to split a group in two, the enums in the new group change
values! That is not a good approach, and was not necessary with the
old system.

This grouping system cannot logically arrange the options into groups,
since it relies on them being attributes of the class, and so you need
to invent a way of grouping. If I had a vote, I'd be strongly -1 on
any proposal with this sort of grouping, it seems dangerous and wrong.


Finally, the proposal seems to concentrate solely on choices as an
enum. The proposal completely ignores that choices can be almost any
kind of value, eg:

MALE="m"
FEMALE="f"
UNKNOWN="?"
GENDER_CHOICES = (
(MALE, "Male"),
(FEMALE, "Female"),
(UNKNOWN, "Unknown"),
  )

this would be an entirely appropriate choices for a CharField.

Cheers

Tom

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Daniel Sokolowski

Hi Lukash, I'm 0 to this.

I find it well thought out but in my use cases I fail to see the advantage 
over the WAY 3 (or WAY 4 pointed out below) and it adds a layer of 
abstraction where a straight tuple or a list does the job well enough.


The choice filtering could come in handy and in the end I don't see why this 
can't co-exist with django's standard way.


WAY 4:

class UserManager(models.Manager):
   GENDER_MALE = 0
   GENDER_FEMALE = 1
   GENDER_NOT_SPECIFIED = 2
   GENDER_CHOICES = (
   (GENDER_MALE, _('male')),
   (GENDER_FEMALE, _('female')),
   (GENDER_NOT_SPECIFIED, _('not specified')),
   )

   def males(self):
   """ Returns all entries accessible through front end site"""
   return self.all().filter(gender=self.GENDER_MALE)
   def females(self):
   """ Returns all entries accessible through front end site"""
   return self.all().filter(gender=self.GENDER_FEMALE)
   ...

class User(models.Model):
   gender = models.IntegerField(choices=UserManager.GENDER_CHOICES,
   default=UserManager.GENDER_NOT_SPECIFIED)
   objects = UserManager()

   ef greet(self):
   if self.gender == UserManager.GENDER_MALE:
   return 'Hi, boy.'
   elif self.gender == UserManager.GENDER_NOT_SPECIFIED:
   return 'Hello, girl.'
   else: return 'Hey there, user!'

-Original Message- 
From: Łukasz Langa

Sent: Wednesday, April 04, 2012 9:15 AM
To: django-developers@googlegroups.com
Subject: Re: Proposal: upgrading the choices machinery for Django

Wiadomość napisana przez Dan Poirier w dniu 4 kwi 2012, o godz. 14:08:


One nice addition would be to specify explicitly what .id a particular
Choice should have. It would be useful in converting existing code that
might not have chosen to number its choices the same way this proposal
does by default. It looks like you could use Choices.Group(n-1) to make
the next choice defined use id=n, but that's not very elegant.


Right. The ability to explicitly set the .id on a single choice is useful.

The only reason it's not already there is that I didn't want to write my own
`makemessages`. Then again I might do just that. My proposal is based on
a working implementation because I didn't want to fall into bikeshedding on
what could have been theoretically implemented.

What we need now to push things forward is a list of +1, +1 if ..., +0,
+0 if ..., -1, "how dare you?!" from a bunch of people.

--
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

--
You received this message because you are subscribed to the Google Groups 
"Django developers" group.

To post to this group, send email to django-developers@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.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Thomas Guettler

Am 03.04.2012 21:31, schrieb Łukasz Langa:

A nice HTML rendering of this proposal is available at:
http://lukasz.langa.pl/2/upgrading-choices-machinery-django/



==
Upgrading the choices machinery for Django
==
...



since most people have something like this in their code, a better solution 
should require
only some additional characters.

GENDER_CHOICES = (
  (0, 'male'),
  (1, 'female'),
  (2, 'not specified'),
  )

# if this would be supported, updating the code would be easy
GENDER_CHOICES = Choices(
  (0, 'male'),
  (1, 'female'),
  (2, 'not specified'),
  )


#about Group
>>> class License(Choices):
  ...   COPYLEFT = Choices.Group(0)
  ...   gpl_any = Choice("GPL, any")

I don't like Choices.Group. I think it is better if choices must be set 
explicit, not
implicit (auto-increment).


Here is my solution, which is similar to way3 in your proposal.

{{{

class ChoiceDict(SortedDict):
'''
Iterating this SortedDict will return
the items (and not the keys). This is handy if
you want to uses this for a choice list on
a model field
'''
__iter__=SortedDict.iteritems

class Foo(models.Model):
STATE_NEW='new'
STATE_DONE='done'
states=ChoiceDict((STATE_NEW, _('new')),
  (STATE_DONE, _('done')),
  )
state=models.CharField(max_length=16, verbose_name=u'Status', 
default=STATE_NEU, choices=states)
}}}
I like this since, you can access the verbose string like this: 
Foo.states[obj.state]


--
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Łukasz Langa
Wiadomość napisana przez Dan Poirier w dniu 4 kwi 2012, o godz. 14:08:

> One nice addition would be to specify explicitly what .id a particular
> Choice should have. It would be useful in converting existing code that
> might not have chosen to number its choices the same way this proposal
> does by default. It looks like you could use Choices.Group(n-1) to make
> the next choice defined use id=n, but that's not very elegant.

Right. The ability to explicitly set the .id on a single choice is useful.

The only reason it's not already there is that I didn't want to write my own
`makemessages`. Then again I might do just that. My proposal is based on
a working implementation because I didn't want to fall into bikeshedding on
what could have been theoretically implemented.

What we need now to push things forward is a list of +1, +1 if ..., +0,
+0 if ..., -1, "how dare you?!" from a bunch of people.

-- 
Best regards,
Łukasz Langa
Senior Systems Architecture Engineer

IT Infrastructure Department
Grupa Allegro Sp. z o.o.

http://lukasz.langa.pl/
+48 791 080 144

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: Proposal: upgrading the choices machinery for Django

2012-04-04 Thread Dan Poirier
I like this - it solves something that's always bugged me.

One nice addition would be to specify explicitly what .id a particular
Choice should have. It would be useful in converting existing code that
might not have chosen to number its choices the same way this proposal
does by default. It looks like you could use Choices.Group(n-1) to make
the next choice defined use id=n, but that's not very elegant.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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.



Proposal: upgrading the choices machinery for Django

2012-04-03 Thread Łukasz Langa
A nice HTML rendering of this proposal is available at:
http://lukasz.langa.pl/2/upgrading-choices-machinery-django/



==
Upgrading the choices machinery for Django
==

Specifying choices for form fields and models currently does not do much justice
to the `DRY
`_
philosophy Django is famous for. Everybody seems to either have their own way of
working around it or live with the suboptimal tuple-based pairs. This Django
enhancement proposal presents a comprehensive solution based on an existing
implementation, explaining reasons behind API decisions and their implications
on the framework in general.

Current status
--

The current way of specifying choices for a field is as follows::

  GENDER_CHOICES = (
  (0, 'male'),
  (1, 'female'),
  (2, 'not specified'),
  )

  class User(models.Model):
  gender = models.IntegerField(choices=GENDER_CHOICES)
  
When I then want to implement behaviour which depends on the ``User.gender``
value, I have a couple of possibilities. I've seen all in production code which
makes it all the more scary.

Way 1
~

Use ``get_FIELD_display``::

  GENDER_CHOICES = (
  (0, 'male'),
  (1, 'female'),
  (2, 'not specified'),
  )

  class User(models.Model):
  gender = models.IntegerField(choices=GENDER_CHOICES)

  def greet(self):
  gender = self.get_gender_display()
  if gender == 'male':
  return 'Hi, boy.'
  elif gender == 'female':
  return 'Hello, girl.'
  else:
  return 'Hey there, user!'

This will fail once you start translating the choice descriptions or if you
rename them and is generally brittle. So we have to improve on it by using the
numeric identifiers.

Way 2
~

Use numeric IDs::

  GENDER_CHOICES = (
  (0, _('male')),
  (1, _('female')),
  (2, _('not specified')),
  )

  class User(models.Model):
  gender = models.IntegerField(choices=GENDER_CHOICES,
  default=2)

  def greet(self):
  if self.gender == 0:
  return 'Hi, boy.'
  elif self.gender == 1:
  return 'Hello, girl.'
  else:
  return 'Hey there, user!'

This is just as bad because once the identifiers change, it's not trivial to
grep for existing usage, let alone 3rd party usage. This looks less wrong when
using ``CharFields`` instead of ``IntegerFields`` but the problem stays the
same.  So we have to improve on it by explicitly naming the options.

Way 3
~

Explicit choice values::

  GENDER_MALE = 0
  GENDER_FEMALE = 1
  GENDER_NOT_SPECIFIED = 2

  GENDER_CHOICES = (
  (GENDER_MALE, _('male')),
  (GENDER_FEMALE, _('female')),
  (GENDER_NOT_SPECIFIED, _('not specified')),
  )

  class User(models.Model):
  gender = models.IntegerField(choices=GENDER_CHOICES,
  default=GENDER_NOT_SPECIFIED)

  def greet(self):
  if self.gender == GENDER_MALE:
  return 'Hi, boy.'
  elif self.gender == GENDER_NOT_SPECIFIED:
  return 'Hello, girl.'
  else: return 'Hey there, user!'

This is a saner way but starts getting overly verbose and redundant. You can
improve encapsulation by moving the choices into the ``User`` class but that on
the other hand beats reusability.

The real problem however is that there is no `One Obvious Way To Do It
`_ and newcomers are likely to choose
poorly.

tl;dr or The Gist of It
---

My proposal suggests embracing a solution that is already implemented and tested
with 100% statement coverage. It's easily installable::

  pip install dj.choices

and lets you write the above example as::

  from dj.choices import Choices, Choice

  class Gender(Choices):
  male = Choice("male")
  female = Choice("female")
  not_specified = Choice("not specified")

  class User(models.Model):
  gender = models.IntegerField(choices=Gender(),
  default=Gender.not_specified.id)

  def greet(self):
  gender = Gender.from_id(self.gender)
  if gender == Gender.male:
  return 'Hi, boy.'
  elif gender == Gender.female:
  return 'Hello, girl.'
  else:
  return 'Hey there, user!'

or using the provided ``ChoiceField`` (fully compatible with
``IntegerFields``)::

  from dj.choices import Choices, Choice
  from dj.choices.fields import ChoiceField

  class Gender(Choices):
  male = Choice("male")
  female = Choice("female")
  not_specified = Choice("not specified")

  class User(models.Model):
  gender = ChoiceField(choices=Gender,
  default=Gender.not_specified)

  def greet(self):
  if self.gender == Gender.male:
  return 'Hi, boy.'
  elif