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

Nested object discriminator field doesn't get serialized to str properly in model_dump #9235

Open
1 task done
kf-novi opened this issue Apr 12, 2024 · 12 comments
Open
1 task done
Labels
bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation
Milestone

Comments

@kf-novi
Copy link
Contributor

kf-novi commented Apr 12, 2024

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

If using a discriminator field for a nested object as seen in the example code, the model_dump function does not serialize that discriminator field from SomeEnum to str.

I would have expected that discriminator field to be serialized automatically.

Example Code

from enum import Enum
from pydantic import BaseModel, Field
from typing import Literal


class SomeEnum(str, Enum):
    A = "a"
    B = "b"
    C = "c"


class InnerClass(BaseModel):
    field: Literal[SomeEnum.A, SomeEnum.B]


class OuterClass(BaseModel):
    inner: InnerClass = Field(discriminator="field")


def test_inner_class():
    inner = InnerClass(field=SomeEnum.A)
    d = inner.model_dump(mode="json")
    assert d["field"] == "a"


def test_outer_class():
    outer = OuterClass(inner=InnerClass(field=SomeEnum.A))
    d = outer.model_dump(mode="json", by_alias=True)

    assert not isinstance(d["inner"]["field"], SomeEnum)
    assert isinstance(d["inner"]["field"], str)
    assert str(d["inner"]["field"]) == "a"

if __name__ == "__main__":
  test_inner_class() # Works as expected
  test_outer_class() # assert error

Python, Pydantic & OS Version

pydantic version: 2.7.0
        pydantic-core version: 2.18.1
          pydantic-core build: profile=release pgo=true
                 install path: /Users/kent/Library/Caches/pypoetry/virtualenvs/pydantic-test-3SUq0Hnt-py3.11/lib/python3.11/site-packages/pydantic
               python version: 3.11.1 (main, Mar 16 2023, 12:51:25) [Clang 13.1.6 (clang-1316.0.21.2.5)]
                     platform: macOS-12.5.1-arm64-arm-64bit
             related packages: typing_extensions-4.11.0
                       commit: unknown
@kf-novi kf-novi added bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Apr 12, 2024
kf-novi pushed a commit to kf-novi/pydantic that referenced this issue Apr 12, 2024
@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 12, 2024

I don't know if this is unexpected or even fixable behavior, but created a PR with failing test case. I may delve into the code and see if there is an easy fix for this and add it for your review. If the PR is unwanted, please close, I won't be offended.

@sydney-runkle
Copy link
Member

@kf-novi,

Thanks for reporting this. I don't actually know if this is a bug - why would you use a discriminator here, if the type isn't a union?

Just a note - this isn't a regression in 2.7.0, but rather has been present throughout recent minor releases.

@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 13, 2024 via email

@sydney-runkle
Copy link
Member

Ah I see, thanks

@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 14, 2024 via email

@sydney-runkle
Copy link
Member

Yep, makes sense. Especially now that we've moved the enum validation logic over there.

@sydney-runkle sydney-runkle added this to the Union Issues milestone Apr 14, 2024
kf-novi added a commit to kf-novi/pydantic that referenced this issue Apr 15, 2024
kf-novi added a commit to kf-novi/pydantic that referenced this issue Apr 15, 2024
@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 19, 2024

This appears to be a regression in pydantic/pydantic-core. This bug does not exist in 2.6.4 but does exist in 2.7.0.

pydantic-core version for my 2.6.4 install is 2.16.3
pydantic-core version for my 2.7.0 install is 2.18.1

@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 19, 2024

Did some further investigation. I got as far back as pydantic-core 2.17.0 with this bug still in place. serialize_as_any was added to to_python in 2.17.0 so that was the farthest back I could easily go in my search for this regression.

So, my guess is this bug was introduced in pydantic-core 2.17.0. Not sure about the particular feature or commit that introduced it. Hopefully that helps track it down!

@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 19, 2024

🤔 I rigged up pydantic-core on my dev box and started rolling back changes. The commit that causes the regression seems to be pydantic/pydantic-core@71d54a2 which is the upgrade to PyO3 0.21 beta.

edit to prevent spam: PyO3 0.21.2 does not fix this issue either. Time to see if I can figure out how to debug rust code.

@kf-novi
Copy link
Contributor Author

kf-novi commented Apr 19, 2024

I've traced the issue to src/serializers/type_serializers/literal.rs

In pydantic/pydantic-core@91c3541 the line in question is 125. Before pyo3 0.21.0 the "inner" value is dog but after, it's SomeEnum.DOG

I don't know why this is happening. I'm having fun tracking this down so I'll see if I figure it out or not.

If I'm abusing the issue comments, please tell me and I'll stop posting new ones and edit the old ones.

edit: As near as I can tell, the issue stems from differing behavior of downcast::<PyString>() between pyo3 0.20.x and 0.21.x.

@sydney-runkle
Copy link
Member

@kf-novi,

No worries at all! Always good to have a live discussion re debugging. Ping me if you have any questions (though admittedly, @davidhewitt will be able to help much more promptly with the pyo3 upgrade stuff).

@kf-novi
Copy link
Contributor Author

kf-novi commented May 2, 2024

@davidhewitt I found the behavior change!

In src/serializers/type_serializers/literal.rs line 92, the previous commit had

return Ok(OutputValue::OkStr(s));

and in the upgrade to pyo3 v0.21, it was changed to

return Ok(OutputValue::OkStr(py_str.clone()))

py_str.to_string() returns "SomeEnum.DOG" but py_str.to_str()?.to_string() returns "dog"

My proposed fix is

return Ok(OutputValue::OkStr(PyString::new_bound(value.py(), s)));

I think @kfreezen will create the PR for this one 😁 you can do with it what you will from there.

edit to add: I feel slightly foolish about the sheer amount of time I spent on this tiny bug. Goes to show you how much people want that "contributor" tag 😜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation
Projects
None yet
Development

No branches or pull requests

2 participants