Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"TextChoices" has no attribute "choices" #1858

Open
Kangaroux opened this issue Dec 5, 2023 · 6 comments
Open

"TextChoices" has no attribute "choices" #1858

Kangaroux opened this issue Dec 5, 2023 · 6 comments
Labels
bug Something isn't working good first issue Good for newcomers stubs Issues in stubs files (.pyi)

Comments

@Kangaroux
Copy link

Kangaroux commented Dec 5, 2023

Bug report

What's wrong

When using an enum created from the functional enum API, mypy complains that the enum doesn't have an attribute choices, however it does.

How is that should be

choices should be a supported attribute when the enum is instantiated.

System information

  • OS:
  • python version: 3.10.12
  • django version: 4.2.8
  • mypy version: 1.7.1
  • django-stubs version: 4.2.7
  • django-stubs-ext version: 4.2.7

Repro

from django.db.models import TextChoices

print(TextChoices("test", "A B C").choices)
repro.py:3:1: error: "TextChoices" has no attribute "choices"  [attr-defined]

If you run the script you see the choices are printed correctly:

$ python repro.py 
[('A', 'A'), ('B', 'B'), ('C', 'C')]
@Kangaroux Kangaroux added the bug Something isn't working label Dec 5, 2023
@intgr intgr added good first issue Good for newcomers stubs Issues in stubs files (.pyi) labels Dec 8, 2023
@ngnpope
Copy link
Contributor

ngnpope commented Dec 13, 2023

Duplicate of #729.

The enum functional API is too dynamic for static type checking. In this case mypy would need to know how to split "A B C" and determine the types of each. Even as a list -- ["A", "B", "C"] -- it would need to iterate over that list.

Under the hood this will effectively look like an enum with no members to mypy and, as such, none of the other stuff built on top of that will work properly. (IIRC it's because mypy's enum plugin just gives up and bails out early.)

If you want to have type checking with enums/choicse, use the non-functional approach to defining enums.

See #729 (comment) for a previous response related to this.

@ngnpope
Copy link
Contributor

ngnpope commented Dec 13, 2023

@intgr I think it'd be worth adding a section to the FAQ to discourage use of the functional API as it seems as though it's not something that will ever be fixable.

@intgr
Copy link
Collaborator

intgr commented Dec 13, 2023

This issue isn't about the dynamic enum members. The report is about the choices property.

Even if we can't teach mypy about the dynamically generated members of enum, fixing choices to have type list[tuple[str, str]] seems straightforward, no?

@intgr
Copy link
Collaborator

intgr commented Dec 13, 2023

But +1 for adding a FAQ entry about the enum members.

@ngnpope
Copy link
Contributor

ngnpope commented Dec 13, 2023

It is a duplicate as it stems from the same issue. I delved into it a little more and the problem is down to how mypy handles using the functional API with an enum subclass.

Try the following:

from django.db import models

Functional = models.TextChoices("Functional", "A B C")  # error: Too many arguments for "Functional" [call-arg]

class Standard(models.TextChoices):
    A = "A"
    B = "B"
    C = "C"

Functional.choices  # error: "TextChoices" has no attribute "choices"  [attr-defined]
Standard.choices  # ok

As referred to in #729 (comment), this is because mypy doesn't support the functional API for subclasses of Enum. It's bailing out here in mypy/semanal_enum.py:

    def check_enum_call(
        self, node: Expression, var_name: str, is_func_scope: bool
    ) -> TypeInfo | None:
        ...
        if fullname not in ENUM_BASES:
            return None
        ...

And looking at the definition for ENUM_BASES here, we see:

# Note: 'enum.EnumMeta' is deliberately excluded from this list. Classes that directly use
# enum.EnumMeta do not necessarily automatically have the 'name' and 'value' attributes.
ENUM_BASES: Final = frozenset(
    ("enum.Enum", "enum.IntEnum", "enum.Flag", "enum.IntFlag", "enum.StrEnum")
)

Now we cannot add enum.EnumMeta here and we're done - that just seems to break things. But if you add "django.db.models.TextChoices" or something like that to the set, it'll likely work. (Note that you have to use a copy of mypy that hasn't been compiled. I checked out the repo and used that with an equivalent piece of code and made it work.)

What you are proposing is adding something for choices on TextChoices when it should be (and already is) defined for ChoicesMeta (or rather _TextChoicesMeta which is shimmed in to ensure types are correct1). Adding this direct to TextChoices would be wrong as that would imply that the choices property is accessible from the member instance when it isn't.

IMO it's just easier to state that use of the functional API is not supported until that issue is addressed in mypy itself as there is a heap of stuff that just doesn't work as expected.

Footnotes

  1. This does make we wonder whether we should be using generics in some way... 🤔

@intgr
Copy link
Collaborator

intgr commented Dec 13, 2023

state that use of the functional API is not supported until that issue is addressed in mypy itself as there is a heap of stuff that just doesn't work as expected.

I would prefer a pragmatic approach and introduce small hacks as needed (within reason of course, taking into account maintenance complexity).

There are lots of things in the Django API that are imprecisely typed or only half-supported due to mypy/Python type system limitations. I'd rather avoid declaring parts of the Django API as unusable in django-stubs if we can help it.

What you are proposing is adding something for choices on TextChoices when it should be (and already is) defined for ChoicesMeta (or rather _TextChoicesMeta which is shimmed in to ensure types are correct[1]). Adding this direct to TextChoices would be wrong as that would imply that the choices property is accessible from the member instance when it isn't.

You're right, I didn't consider that choices should not be accessible via instance. But even with that caveat, I think adding it would already be a net improvement.

I think this issue could also be solved, by defining a @_classonlyproperty decorator with magic __get__ method that only accepts classes (similar to model field Field.__get__(self, instance: None, ...)).

Of course it could be that I'm missing something and this doesn't work out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers stubs Issues in stubs files (.pyi)
Development

No branches or pull requests

3 participants