agritheory/edgewise

Q: why attrs instead of Pydantic?

Opened this issue · 7 comments

adriangb commented

This is really just a question.

I realize that attrs can be faster (with catters), but on the flip side Pydnatic is used by FastAPI, which IMO is currently the best way to make application backends in Python.

agritheory commented

I have not used pydantic. When I started this project attrs was the stable thing and pydantic was still the new hotness. I haven't been given an overwhelming reason why one is better than the other. The ORM side of things does not require type validation (because data is typed coming from EdgeDB), which is often not case when deserializing from an API. I'm not sure they're incompatible either.

adriangb commented

Agreed on data validation not being necessary when coming from a DB. Pydantic has the ability to construct models that have validation without validating. So you can have data coming from the DB create objects without validation and return those directly to clients via API while using the same ORM to deserialize and validate incoming JSON.

I'm not sure they're incompatible either

This is an interesting idea I had not thought of. It is quite possible that you can dump an attrs object into a Pydantic Model, however that does still mean that you need to separately define your Pydantic Model. You wouldn't be able to make the DB schema the single source of truth.

agritheory commented

One of the ideas I'm most pleased with is using EdgeDB's schema to define python classes/models. The dump attrs-into-pydantic path doesn't excite me or probably anybody else.
I think it would be possible to check for an attrs/pydantic flag. I imagine the inheritance pieces would be very similar so you could add class methods with the same decorator.

adriangb commented

I think it would be possible to check for an attrs/pydantic flag

I think that would be very nice. I'm sure there are some advantages to attrs that I am unaware of. I haven't peeked at your internals so I don't know how feasible this is to implement, but if it is feasible, it would be very nice and would make EdgeDB play very nicely with FastAPI.

agritheory commented

The majority of the work would take place here:
https://github.com/agritheory/edgewise/blob/testing/edgewise/registry.py

A flag would need to be added to merge_class and then do the pydantic version of _build_attributes. I don't know pydantic well enough to know if there should be versions of the scalar pieces as well. I do think that it makes sense to tackle #6 first so that we're working with JSON output from EdgeDB rather than some of the internal edgedb-python classes which I perceive are a little bit of a moving target.

adriangb commented
agritheory commented

In the context of the registry, its less about the serialization/deserialization problem than than correctly creating a model on the fly.

Here's what would need to implemented in the pydantic version of _build_attributes.
https://pydantic-docs.helpmanual.io/usage/models/#dynamic-model-creation