Opened 6 years ago

Closed 6 years ago

#32460 closed Bug (fixed)

TextChoices/IntegerChoices can not have a member with name `label`

Reported by: elonzh Owned by: Nick Pope
Component: Database layer (models, ORM) Version: 3.0
Severity: Normal Keywords: Choices
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

import enum
from django.db.models import TextChoices, IntegerChoices


class A(enum.Enum):
    label = "label"


print("case A, builtin Enum type:", A.label)


class B(TextChoices):
    LABEL = "label"


print("case B, TextChoices with a choice `LABEL`:", B.LABEL)

try:
    class C(TextChoices):
        label = "label"

except Exception as e:
    print("case C, TextChoices with a choice `label`:", e)


try:
    class D(IntegerChoices):
        label = 0

except Exception as e:
    print("case D, IntegerChoices with a choice `label`:", e)

output:

case A, builtin Enum type: A.label
case B, TextChoices with a choice `LABEL`: label
case C, TextChoices with a choice `label`: Cannot reassign members.
case D, IntegerChoices with a choice `label`: Cannot reassign members.

Change History (7)

comment:1 by starryrbs, 6 years ago

Owner: changed from nobody to starryrbs
Status: new → assigned

I checked the source code of django, in django.db.models.enums path:

cls.label = property(lambda self: cls._value2label_map_.get(self.value))
cls.do_not_call_in_templates = True

label and do_not_call_in_templates cannot be used as enum keys.
We may need to add this to the Django document or change the label to configurable.
I submitted a pull request to support this feature. ​https://github.com/django/django/pull/14020

comment:2 by Nick Pope, 6 years ago

Has patch: set
Owner: changed from starryrbs to Nick Pope
Triage Stage: Unreviewed → Accepted

Hi. Thanks for the report.

So looking at this, I don't think that making it possible to customise the .label property name is the right approach.

Enum doesn't have a problem providing .name or .value attributes on the members while still allowing those names to be used as member names.
I have something sorted out that will fix this for .label and .do_not_call_in_templates (although the latter is rather niche!)

This is also broken for .names, .values, .labels, and .choices. These are, however, probably not fixable as they must exist on the top-level class itself and not the members, so they would collide with the member names. Perhaps this part of the problem can be addressed with some documentation.

Alternate ​PR.

in reply to:  2 ; comment:3 by elonzh, 6 years ago

Replying to Nick Pope:

Hi. Thanks for the report.

So looking at this, I don't think that making it possible to customise the .label property name is the right approach.

Enum doesn't have a problem providing .name or .value attributes on the members while still allowing those names to be used as member names.
I have something sorted out that will fix this for .label and .do_not_call_in_templates (although the latter is rather niche!)

This is also broken for .names, .values, .labels, and .choices. These are, however, probably not fixable as they must exist on the top-level class itself and not the members, so they would collide with the member names. Perhaps this part of the problem can be addressed with some documentation.

Alternate ​PR.

using methods to get such members? like get_labels(), these methods introduce API changes but I think it's a clear solution?

in reply to:  3 comment:4 by Nick Pope, 6 years ago

Replying to elonzh:

using methods to get such members? like get_labels(), these methods introduce API changes but I think it's a clear solution?

Every example in the ​enum documentation and the ​Field.choices documentation uses the convention of uppercase member names. Maybe I was naive to assume that people wouldn't use lowercase member names.

While we could rename these, I'm not sure it is worth the change. We'd need to deprecate first for a few releases. It will annoy lots of people who will then have to update this for a small edge case. On top of that, the names get_labels, etc. would also then not be allowed, merely shifting the problem. At the end of the day, these choices helper classes are not compulsory and, as documented, a sequence can be used.

comment:5 by Mariusz Felisiak, 6 years ago

Triage Stage: Accepted → Ready for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

In 41e39c41:

Refs #32460 -- Doc'd and tested that property names of model choice enums cannot be used as members.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

Resolution: → fixed
Status: assigned → closed

In a96c7304:

Fixed #32460 -- Allowed "label"/"do_not_call_in_templates" members in model choice enums.

Note: See TracTickets for help on using tickets.
Back to Top