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

Support Pydantic 2.0 #279

Closed
xl0 opened this issue Jul 12, 2023 · 6 comments
Closed

Support Pydantic 2.0 #279

xl0 opened this issue Jul 12, 2023 · 6 comments

Comments

@xl0
Copy link
Contributor

xl0 commented Jul 12, 2023

Pydantic 2.0 removed create_model_from_typeddict() without a warning, which breaks pyairtable.

Can't say I agree with them just yanking functions without a deprecation warning, but that's on them.

I never used TypedDict. Is there a reason to use it instead of BaseModel, since we already depend on pydantic? @mesozoic , are you ok if I move the types to BaseModel?

@mesozoic
Copy link
Collaborator

We use TypedDict to represent record and field dicts that get returned by Airtable. I don't think it's always practical for us to convert those into models, and it would be a significant change for library users whose code expects those objects as dicts. (Maybe in 3.0)

I do think there's a way to make this library work with both v1 and v2 (it involves conditional imports) but I think we can take our time doing it. As you pointed out, Pydantic 2 made a lot of breaking API changes, so I doubt many people are in a hurry to switch.

(FWIW I do have a couple branches in the works which use BaseModel for net new data structures, like comments and webhooks. Will post those soon.)

@xl0
Copy link
Contributor Author

xl0 commented Jul 12, 2023

I see.

While the TypedDict code has not been around for a pyairtable release yet, people do expect something that behaves like a dict, right?

Would you entertain the idea of returning a subclasses of

class DictBaseModel(BaseModel):
    def __getitem__(self, key):
        return getattr(self, key)

instead of TypedDict?

@mesozoic
Copy link
Collaborator

I think we can entertain any idea as long as it demonstrates value. Does this change help us add any new features or capabilities to the library?

@xl0
Copy link
Contributor Author

xl0 commented Jul 24, 2023

I feel the main advantage is, Pydantic is a lot more popular than TypedDict, which means that

  1. Everyone knows how to use it.
  2. Oher libraries and tools integrate well with it.

Since PyAirtable already depends on Pydantic internally, it would make sense to also use it for the interface.
Pydantic is probably going to be the way forward for ORM. It would be confusing if part of PyAirtable interface is in Pydantic, and another part is in TypedDict.

And now would be a good time for this switch, before the TypedDict implementation hit a release. I think the change would be very straightforward. If you feel like it's worth it, I can implement it.

I have not used TypedDict before, so I don't know its limitations compared to Pydantic.

@mesozoic
Copy link
Collaborator

TypedDict is part of the Python standard library; you can read more about it here.

I don't think it's confusing for parts of the pyAirtable API, like table.all(), to return dict, especially when those data structures are more or less "raw" passthroughs of what the Airtable API returns itself. Every Python developer knows how to deal with a dict and understands their behavior. Far fewer are familiar with Pydantic models and their quirks.

At this point the primary argument for not changing the return type of table.all() is for backwards compatibility. If we're going to replace that with Pydantic model instances (which implement __getitem__, maybe) then there's got to be some clear benefit in the form of new library capabilities. If there are specific examples of how this change might make it easier to integrate pyAirtable with other libraries, I'm all for hearing those.

For now I think the scope of this issue should be limited to supporting both Pydantic v1 and v2 (so we can minimize dependency conflicts) or, if that proves impossible, migrating to Pydantic v2 once it's clear that the older version of the library is no longer the dominant dependency (but we're not there yet).

@mesozoic
Copy link
Collaborator

Resolved in #288 🙌

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