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

Pydantic V2 Support #2978

Closed
thejaminator opened this issue Jul 25, 2023 · 8 comments
Closed

Pydantic V2 Support #2978

thejaminator opened this issue Jul 25, 2023 · 8 comments
Assignees

Comments

@thejaminator
Copy link
Contributor

thejaminator commented Jul 25, 2023

To try it out,

pip install strawberry-graphql==0.196.0.dev.1690222024

Please report any problems faced migrating in this mega issue

Current yet to be merged MR here

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@thejaminator thejaminator self-assigned this Jul 25, 2023
@rgon
Copy link

rgon commented Jul 25, 2023

Support for from_pydantic and to_pydantic is not implemented yet.

It's defined in the original v1 implementation in:

has_custom_from_pydantic = hasattr(
cls, "from_pydantic"
) and cls.from_pydantic.__qualname__.endswith(f"{cls.__name__}.from_pydantic")
has_custom_to_pydantic = hasattr(
cls, "to_pydantic"
) and cls.to_pydantic.__qualname__.endswith(f"{cls.__name__}.to_pydantic")

if not has_custom_from_pydantic:
cls.from_pydantic = staticmethod(from_pydantic_default)
if not has_custom_to_pydantic:
cls.to_pydantic = to_pydantic_default

and there are also some references to it in strawberry/experimental/pydantic/conversion.py#L62, _convert_from_pydantic_to_strawberry_type and in convert_strawberry_class_to_pydantic_model for to_pydantic.

However, none are present in thejaminator's fork

I suppose we could just add it back in. It also seems like there aren't any tests for this functionality, since all seem to have passed.

@patrick91
Copy link
Member

@rgon would you be interested in adding those tests? 😊 (also, thanks a lot for testing the pre-release!)

@thejaminator
Copy link
Contributor Author

thejaminator commented Jul 25, 2023

Support for from_pydantic and to_pydantic is not implemented yet.

Thanks alot for testing it out! Appreciate it!
Also note that my fork reuses the v1 strawberry pydantic code, so it'll still run the code that you quoted :)

This test should work. Could you try it out?

def test_can_convert_input_types_to_pydantic():
    class User(BaseModel):
        age: int
        password: Optional[str]

    @strawberry.experimental.pydantic.input(User)
    class UserInput:
        age: strawberry.auto
        password: strawberry.auto

    data = UserInput(age=1, password=None)
    user = data.to_pydantic()

    assert user.age == 1
    assert user.password is None

Are you getting the errors at runtime or due to mypy / pyright?

Or are you talking about the specific custom redefinition of has_custom_to_pydantic?

@rgon
Copy link

rgon commented Jul 25, 2023

@patrick91 I will have to take a shot at this. Not an expert at that, but will take a shot at this in a few days if you don't mind a little post-PR guidance :)

@thejaminator no worries! Yes, I'm talking about defining custom from_pydantic and to_pydantic methods as described here in the docs, in 'custom conversion logic' (whoops I linked to a different section initially, sorry for the confusion). I'm specifically talking about defining methods like this one (copied from the docs):

@strawberry.experimental.pydantic.type(model=User)
class UserType:
    id: strawberry.auto
    # Flatten the content dict into specific fields in the query
    content_name: Optional[str] = None
    content_description: Optional[str] = None

    @staticmethod
    def from_pydantic(instance: User, extra: Dict[str, Any] = None) -> "UserType":
        data = instance.dict()
        content = data.pop("content")
        data.update({f"content_{k.value}": v for k, v in content.items()})
        return UserType(**data)

    def to_pydantic(self) -> User:
        data = dataclasses.asdict(self)

        # Pull out the content_* fields into a dict
        content = {}
        for enum_member in ContentType:
            key = f"content_{enum_member.value}"
            if data.get(key) is not None:
                content[enum_member.value] = data.pop(key)
        return User(content=content, **data)

I see no runtime/mypy errors. However, when implementing a custom 're'-serializer of sorts, with this pre-release these methods had no effect (not even print statements were called). Tracing the function back, I noticed that the check for custom-defined conversion methods is not yet implemented.

The test you sent works perfectly. to_pydantic does indeed work, but redefining it doesn't. I think it can just be supported with a has_custom_from_pydantic check like in the v1 release.

@thejaminator
Copy link
Contributor Author

thejaminator commented Jul 26, 2023

@rgon thank you!

Tracing the function back, I noticed that the check for custom-defined conversion methods is not yet implemented.

Thats weird. because I havent changed anything at all for that logic. it should be the same as V1. Not sure why that worked before but it doesn't now

Besides the custom to_pydantic function, are there other issues you've faced? If not, I can merge in the current PR earlier :)

@AlexTo
Copy link

AlexTo commented Aug 4, 2023

is there a version that works with Pydantic v2 yet? :D very much appreciated !!!

@patrick91
Copy link
Member

@AlexTo we just released one! Feel free to open an issue if you find bugs! I'll close this issue 😊

@AlexTo
Copy link

AlexTo commented Aug 7, 2023

@patrick91 that's awesome, will update right away, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants