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 2 @computed_field not detected #1258

Closed
andreasnuesslein opened this issue Jun 29, 2024 · 6 comments
Closed

Pydantic 2 @computed_field not detected #1258

andreasnuesslein opened this issue Jun 29, 2024 · 6 comments

Comments

@andreasnuesslein
Copy link

Hi there!,

first of: thanks so much for this package :)

So, I'm using the @extend_schema_field and everything works fine except @computed_field is not detected

To Reproduce

from pydantic import BaseModel
class TestModel(BaseModel):
    number: int

    @computed_field
    @property
    def twice(self) -> bool:
        return number * 2

The resulting schema.yaml will not include "twice"

If it helps, I can try to create a PR.

Cheers

@tfranzel
Copy link
Owner

Hi @andreasnuesslein

So we actually don't do that much with pydantic. We use the schema generated by pydantic and just shape it a bit. But you're welcome to investigate why exactly this does not show up. Here is the code handling pydantic:

https://github.com/tfranzel/drf-spectacular/blob/master/drf_spectacular/contrib/pydantic.py

Let me know what you find out even if you decide not to do a PR. thanks

@andreasnuesslein
Copy link
Author

Hi @tfranzel thanks for your response!

so I investigated a bit and the solution seems simple.

According to this: pydantic/pydantic#6298 (comment)

it'd be:

- schema = model_json_schema(self.target, ref_template="#/components/schemas/{model}")
+ schema = model_json_schema(self.target, mode='serialization', ref_template="#/components/schemas/{model}")

which for me yields in this case the right result (difference):
image

However I dont really know if the implications are bigger than just that...

@tfranzel
Copy link
Owner

tfranzel commented Jul 1, 2024

thanks for investigating @andreasnuesslein.

So as a matter of fact we already have an open PR (#1156) for this change, even though for a different reason.

I think with your point on top, it now makes it so that the benefits outweigh the potential issues of the change.

@andreasnuesslein
Copy link
Author

@tfranzel ps: which cableway site is that in your buddy icon? reminds me a bit of Rossau.

@tfranzel
Copy link
Owner

tfranzel commented Jul 1, 2024

hehe, that picture was taken in Niederweimar near Marburg.

@andreasnuesslein
Copy link
Author

andreasnuesslein commented Jul 2, 2024

🤙 okay, yeah have not been there 🙈 looks nice though

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

2 participants