django-choices icon indicating copy to clipboard operation
django-choices copied to clipboard

ChoiceItem naming convention

Open tedmiston opened this issue 8 years ago • 11 comments

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:

class Person(models.Model):
    # Choices
    class PersonType(DjangoChoices):
        Customer = ChoiceItem("C")
        Employee = ChoiceItem("E")
        Groundhog = ChoiceItem("G")
    ...

From docs/index.rst:

class Book(models.Model):

    class BookTypes(DjangoChoices):
        short_story = ChoiceItem('short', 'Short story')
        novel = ChoiceItem('novel', 'Novel')
        non_fiction = ChoiceItem('non_fiction', 'Non fiction')
    ...

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

tedmiston avatar Mar 09 '16 04:03 tedmiston

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
  • snake_case
  • UPPERCASE

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 called DjangoChoices 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!

sergei-maertens avatar Mar 09 '16 09:03 sergei-maertens

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.


PEP 435

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.

PEP 354

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.

Earlier

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 itself

Finally, I decide we'll look back to enum in the Python 3 docs, but even it uses multiple styles.

Ex. 8.13.2. Creating an Enum

from enum import Enum
class Color(Enum):
    red = 1
    green = 2
    blue = 3

Ex. 8.13.13.4. Planet

class Planet(Enum):
    MERCURY = (3.303e+23, 2.4397e6)
    VENUS   = (4.869e+24, 6.0518e6)
    ...

Conclusion

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.

tedmiston avatar Mar 09 '16 22:03 tedmiston

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.

sergei-maertens avatar Mar 09 '16 22:03 sergei-maertens

This makes sense too. I'll sit tight for a few days to let it sink in.

tedmiston avatar Mar 10 '16 18:03 tedmiston

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?

sergei-maertens avatar Mar 29 '16 15:03 sergei-maertens

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 avatar Mar 29 '16 18:03 juanjbrown

@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 ChoiceItems 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 avatar Mar 30 '16 11:03 sergei-maertens

@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.

juanjbrown avatar Mar 30 '16 16:03 juanjbrown

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é" [email protected] 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

sergei-maertens avatar Mar 30 '16 16:03 sergei-maertens

@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 use singular style.

tedmiston avatar Mar 31 '16 19:03 tedmiston

Awesome! Let's follow their example then and stick with singular. On Mar 31, 2016 21:39, "Taylor Edmiston" [email protected] 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

sergei-maertens avatar Mar 31 '16 19:03 sergei-maertens

Closing this issue since Django now provides native choices. I'd recommend to follow whatever convention Django uses in its own documentation.

sergei-maertens avatar Jul 24 '23 16:07 sergei-maertens