Closed tedmiston closed 1 year ago
Hi Taylor, you're raising a good point, recently we had a similar discussion in our company. I'll outline the considerations.
There are actually three naming styles that are possible:
CamelCase
In Python, CamelCase indicates a class. I think this is wrong, since the choices are 'regular' attributes on the DjangoChoices
subclass, and they are instances of the ChoiceItem
class, not a class by themselves.
snake_case
Personally, I think this is the right choice, for the reasons outlined above. It's also nice that it's in line with the Python 3 Enum
like you pointed out. Semantically, it makes sense.
UPPERCASE This is in line with how Django itself defines the choices, as constants. A choice is strictly speaking a constant, so I'd prefer uppercase in favour of CamelCase.
So, to summarize: snake_case would be the preferred way, followed by uppercase and then camelcase.
(Side thought: _Type vs _Types) Good one, I often find myself doubting between the singular and plural form as well.
Enum
does it with singular, Django itself does it with plural form (FOO_BAR_CHOICES
). I'd say to stick with plural, since the base class is calledDjangoChoices
as well.I think as a user of the library it would be simpler if there were one obvious right way to avoid having to guess. I'm happy to contribute a PR.
Agree, a consistent style avoids confusion and if there are internal discussions in companies, you can refer back to the official documentation of the library.
A PR would be greatly appreciated!
Cool. This raises a great point. I guess the reason CamelCase "felt right" is that I'm thinking about them like subclasses, but you're right -- that's not what they are.
I think you're not alone with UPPERCASE, because after all, we're defining constants (or at least pseudo-constants), and that is consistent with PEP 8.
Thank you for digging into Django as a point of reference. Based on this I was curious to dig deeper into Python. Warning: long post ahead.
Particularly, after confirming there is no guideline for this in PEP 8, I started with PEP 0435 -- Adding an Enum type to the Python standard library. The convention used throughout the document is snake_case, though it is never explicitly called out or prescribed. So then I wondered: Where did it come from?
Also, the discussion mentions a previously rejected PEP for enums and also Guido's proposal to just add flufl.enum
to stdlib. Okay, let's look there.
flufl.enum
In its examples, flufl.enum
uses snake_case. I still found this odd because we define an enum as a collection of related constants. The documentation is consistent with this definition even if the naming convention is not:
An enumeration is a set of symbolic names bound to unique, constant values, called enumeration items.
and
The properties of an enumeration are useful for defining an immutable, related set of constant values that have a defined sequence but no inherent semantic meaning.
So if they're constants, we should name them LIKE_A_CONSTANT
, right?
The motivation for flufl.enum mentions that it is lifted from PEP 354, the original (rejected) PEP for enums.
So I dug into PEP 0354 -- Enumerations in Python. It also uses snake_case, but digging deeper there's a note that it was inspired by something else:
This design is based partly on a recipe [2] from the Python Cookbook.
Where [2] is First Class Enums in Python written by Zoran Isailovski in 2005, using an abbreviated CamelCase convention.
Zoran's snippet uses this style:
...
Days = Enum('Mo', 'Tu', 'We', 'Th', 'Fr', 'Sa', 'Su')
print Days
print Days.Mo
...
That implementation also makes reference to two earlier implementations: One by Will Ware in 2001 using UPPERCASE; the other by Samuel Reynolds in 2004 using CamelCase.
(At this point I tell myself we've come full circle.)
enum
itselfFinally, I decide we'll look back to enum
in the Python 3 docs, but even it uses multiple styles.
from enum import Enum
class Color(Enum):
red = 1
green = 2
blue = 3
class Planet(Enum):
MERCURY = (3.303e+23, 2.4397e6)
VENUS = (4.869e+24, 6.0518e6)
...
After all this, I'm somewhat split between snake_case and UPPERCASE.
I think UPPERCASE better represents constant-ness, but for a reason I wasn't able to find, the community at some point picked up using snake_case for naming constants defined inside enums.
Of course, as you've shown Django chooses to use UPPERCASE in their own examples. And since this is a Django-specific library, I feel inclined to follow their convention, especially having explicitly documented rationale.
Perhaps it's just that the greater Python community hasn't questioned the enum naming convention in detail.
I'm not sure whether this changes your conclusion, but at least I feel more informed.
It's great that you dug into it like that, and your conclusion is a solid one.
However... (yeah, ofcourse...), I'm not entirily convinced. The way that Django does it - proper constants - feels like it's mostly to separate it from regular class attributes and model fields, and not pollute the (sub)class namespace(s). I have no references for this, but if we look at the user model, the extra 'configuration' attributes are also uppercase, because they're constants as well I'd suppose.
With DjangoChoices, there's not much need any more to make them uppercase in terms of namespace pollution, there's only a single Choices class left for many possible choices. It's also clear that it's a class that does some 'special' things, and not a simple constant (scalar or tuple/lust). So, I'm still inclined to use snake_case.
I'll have to let this sink in for a bit though, both options make sense to me.
This makes sense too. I'll sit tight for a few days to let it sink in.
I forgot about this issue for a while, but I think I still prefer snake_case for the naming convention, since it's easier reading and not as 'shouty' as uppercase choices.
@tedmiston would you still provide the PR for the documentation fixes, or should I do it?
So in the way @sergei-maertens sees it, "feels like it's mostly to separate it from regular class attributes and model fields, and not pollute the (sub)class namespace(s)." snake_case makes sense, but I see snake_case as something that might change. The option SNAKE_CASE tells me that it’s just a reference to something shouldn’t change. The same way that Python doesn’t really have private methods but it has the convention that tells you that a method is for internal class usage (do vs _do). Personally, I would go with SNAKE_CASE because it's normally used as a constant.
For the plural vs singular in the choices class name, I would go with singular
class Planet(DjangoChoices ):
PLUTO = ChoiceItem("P")
But that's just because I see it like the names of the tables in a DB.
@juanjbrown I don't think that reasoning holds a 100%, if you look at CBV in Django for instance, there are certain 'configuration' attributes that are contant-y as well, but still lowercased. Looking at an UpdateView
for example, you have the model
and form_class
attributes, which are purely configuration.
The point of the DjangoChoices
ChoiceItem
s is also that you can potentially change the actual values and labels, without having to update your entire codebase for them, because they are referenced through the DjangoChoices
attributes.
@sergei-maertens that's true. hm... The way I saw it was that if I have something like:
class Planet(object):
PLUTO = 'P'
Then having the choices would be:
class Planet(Choices):
PLUTO = ChoiceItem('P')
Because after initializing Planet
, Planet.PLUTO
ends up being 'P'
and not an instance of ChoiceItem
.
Well, you are of course free to follow your own conventions, it's clear from this duscussion that there doesn't seem to be One True Style, and your reasoning is definitely not absurd. However, the important thing for this library now is that the documentation uses a consistent style. If you decide to divert from that style in your own projects and have a motivation for your collaborators/colleagues, that's great, stick to that decision :-) On Mar 30, 2016 18:44, "Juan José" notifications@github.com wrote:
@sergei-maertens https://github.com/sergei-maertens that's true. hm... The way I saw it was that if I have something like:
class Planet(object): PLUTO = 'P'
Then having the choices would be:
class Planet(Choices): PLUTO = ChoiceItem('P')
Because after initializing Planet, Planet.PLUTO ends up being 'P' and not an instance of ChoiceItem.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/bigjason/django-choices/issues/32#issuecomment-203519954
Awesome! Let's follow their example then and stick with singular. On Mar 31, 2016 21:39, "Taylor Edmiston" notifications@github.com wrote:
@sergei-maertens https://github.com/sergei-maertens Sure, I'm happy to send up the PR. Do you want to standardize on singular or plural names for Choices objects in this too (BookType or BookTypes)?
A cursory search shows that django-model-utils and dj.choices both https://django-model-utils.readthedocs.org/en/latest/utilities.html#choices use https://github.com/ambv/dj.choices/ singular style.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/bigjason/django-choices/issues/32#issuecomment-204093814
Closing this issue since Django now provides native choices. I'd recommend to follow whatever convention Django uses in its own documentation.
I've noticed there are two naming styles used: CamelCase and snake_case.
For example,
Person.PersonType.Customer
vs.Book.BookTypes.short_story
.(Side thought: _Type vs _Types)
From
README.rst
:From
docs/index.rst
:Personally I think camel casing "feels more right" given that I'm interacting with the choice items instances like a class type as opposed to like variables. (And this is the convention I've followed personally.)
But enum in Python 3 takes the other approach.
I'd like to hear your thoughts on which style you'd prefer. I think as a user of the library it would be simpler if there were one obvious right way to avoid having to guess. I'm happy to contribute a PR.
-Taylor