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

Literal enum fields are stored as mix-in type instead of enum field when using parse_raw or parse_json #2166

Closed
4 tasks done
yobiscus opened this issue Dec 1, 2020 · 4 comments · Fixed by #2181
Closed
4 tasks done

Comments

@yobiscus
Copy link

yobiscus commented Dec 1, 2020

Checks

  • I added a descriptive title to this issue
  • I have searched (google, github) for similar issues and couldn't find anything
  • I have read and followed the docs and still think this feature/change is needed
  • After submitting this, I commit to one of:
    • Look through open issues and helped at least one other person
    • Hit the "watch" button on this repo to receive notifications and I commit to help at least 2 people that ask questions in the future
    • Implement a Pull Request for a confirmed bug

Feature Request

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.7.1
            pydantic compiled: True
                 install path: /home/user/project/.venv/lib/python3.8/site-packages/pydantic
               python version: 3.8.5 (default, Jul 28 2020, 12:59:40)  [GCC 9.3.0]
                     platform: Linux-5.4.0-53-generic-x86_64-with-glibc2.29
     optional deps. installed: ['typing-extensions']
import enum
from typing import Literal
from pydantic import BaseModel

class Bar(str, enum.Enum):
    FIZ = "fiz"
    FUZ = "fuz"

class Foo(BaseModel):
    bar: Bar
    barfiz: Literal[Bar.FIZ]

Foo.parse_raw('{"bar": "fiz", "barfiz": "fiz"}')
Foo.parse_obj({"bar": "fiz", "barfiz": "fiz"})
# both return: Foo(bar=<Bar.FIZ: 'fiz'>, barfiz='fiz')

# expected: Foo(bar=<Bar.FIZ: 'fiz'>, barfiz=<Bar.FIZ: 'fiz'>)

I was expecting that the Pydantic would store Bar.FIZ for the Literal[Bar.FIZ] field above, as opposed to storing a str type (the enum's mix-in type. Mypy flags Literal[Bar.FIZ] above as an enum field, so accessing value on the barfiz attribute, for example, is perfectly fine by Mypy's rules, but breaks during runtime.

I realize that mypy's support for Literal enum fields is relatively new, but they have interesting properties that I am looking to make use of (I would love to see this work with discriminated union types, for example).

Would you consider adding a Config field to enable this behaviour? Should this be flagged as a bug instead and targetted for a 2.0 release?

Possibly related bugs:

@PrettyWood
Copy link
Member

Hi @yobiscus
I feel like the issue is just related to the check done in literal where 'fiz' in {<Bar.FIZ: 'fiz'>} is True.
We could maybe do something like

 def make_literal_validator(type_: Any) -> Callable[[Any], Any]:
     permitted_choices = all_literal_values(type_)
-    allowed_choices_set = set(permitted_choices)

     def literal_validator(v: Any) -> Any:
-        if v not in allowed_choices_set:
+        for x in permitted_choices:
+            if x == v:
+                return x
+        else:
             raise errors.WrongConstantError(given=v, permitted=permitted_choices)
-        return v

     return literal_validator

to return the original value in Literal instead of the passed one but that would cost a bit of performance. There may be a better solution that's just the first one that came in mind.

@yobiscus
Copy link
Author

yobiscus commented Dec 7, 2020

I see, thanks. This now makes sense given the logic. Now that I've had time to digest this some more, I feel like this is a bug in Pydantic (especially because the field is identified by mypy as an enum field rather than a string).

Do you think any other types are subject to this issue? My guess is that this behaviour is unusual and specific to Enums because "fiz" == Bar.FIZ evaluates to True even though isinstance("fiz", Bar) evaluates to False.

I would be happy to look more closely at a solution for this problem, but I am worried about breaking backwards compatibility in a 1.X release, so I'm also wondering what the recommended direction is for this issue.

@PrettyWood
Copy link
Member

Hi @yobiscus
Honestly I feel like this is a bug too. And it can apply to a lot of classes with tolerant __eq__ check though the StrEnum is probably the most common one (__eq__ from str and from Enum).
I opened a PR with the code I showed you.
If you find a better way to do it or think it can break things, feel free to react to it!! 👍
Have a great day

PrettyWood added a commit to PrettyWood/pydantic that referenced this issue Dec 7, 2020
@samuelcolvin
Copy link
Member

Thanks for reporting this, it's definitely a bug.

Another example would be 1 and True (and of course 0 and False) since True in {1, 2} is True. You could also come up with numerous other such errors with custom types and probably other builtin types I can't think of.

I think @PrettyWood's solutions in #2181 is pretty good.

samuelcolvin pushed a commit that referenced this issue Jan 1, 2021
#2181)

* fix: ensure to always return one of the values in `Literal` field type

closes #2166

* perf: improve `literal_validator` speed

Thanks to @yobiscus

* fix: when more options in Literal

switch from `set` to `dict` to still have a O(1) complexity
Thanks @layday :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants