Closed adsharma closed 3 years ago
You can already create a serializer using DataclassSerializer(dataclass=Person)
. What advantage do you see generated serializer classes having above that?
I'm interested in exploring the potential to use fquery to express django views instead of django.db.models.Q()
https://github.com/adsharma/fquery/blob/master/tests/test_operators.py
Queries are written using Object/Graph model, but can be mapped to SQL.
I've used the decorator approach elsewhere such as:
https://github.com/adsharma/fquery/blob/master/tests/mock_user.py https://github.com/ppinard/dataclasses-sql/issues/4
My goal is to try this approach out with django-todo and see if I can create a workflow such as using jupyter notebook to write fqueries, which subsequently result in django views.
For trivial cases, using a decorator vs DataclassSerializer(dataclass=Person)
is not a big deal. But for a non trivial project, I was thinking that using decorators on top of dataclasses consistently would be a usability benefit.
Apart from REST, I'm also looking at GraphQL. I don't yet know how to generate GraphQL using a decorator. Have been doing it manually:
https://github.com/adsharma/fquery/blob/master/tests/graphql_mock_user.py
For trivial cases, using a decorator vs DataclassSerializer(dataclass=Person) is not a big deal. But for a non trivial project, I was thinking that using decorators on top of dataclasses consistently would be a usability benefit.
What's the usability benefit though? Yes, it's a little bit less typing, but you also lose all static analysis/IDE support as they won't know of your magically defined serializers classes. Is there any other benefit?
Given that dataclass itself is a decorator and IDEs and static analysis tools (such as mypy) support it, it should be possible to support other decorators as well. Agree it's more work, but there is end user benefit as described below.
The other benefit of your serializer work is the validation of the input data. My bias is to use the type checker to do more validation than they do right now (e.g. an integer between 1-100 or an enum that gives you 5 options).
This way all metadata relevant to query execution about "Person" is available in the dataclass itself and fquery doesn't need to look at other classes.
On Fri, Dec 11, 2020, 10:15 AM Oxan van Leeuwen notifications@github.com wrote:
For trivial cases, using a decorator vs DataclassSerializer(dataclass=Person) is not a big deal. But for a non trivial project, I was thinking that using decorators on top of dataclasses consistently would be a usability benefit.
What's the usability benefit though? Yes, it's a little bit less typing, but you also lose all static analysis/IDE support as they won't know of your magically defined serializers classes. Is there any other benefit?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oxan/djangorestframework-dataclasses/issues/33#issuecomment-743347797, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFA2A525SAOZH53CEUKURDSUJOVRANCNFSM4UW6HZKQ .
Given that dataclass itself is a decorator and IDEs and static analysis tools (such as mypy) support it, it should be possible to support other decorators as well. Agree it's more work, but there is end user benefit as described below.
The big difference is that your proposed decorator magically defines another class, as opposed to basically just modifying the behaviour of the class the decorator is applied to. Even if the analyser doesn't support the dataclass decorator, it still recognizes the dataclass type and the fields defined on it. This decorator will require explicit support in analysers, which given that it's a relatively specific 3rd party library I don't see happening.
That magic class definition is the main reason I'm not too fond of this. It's really unexpected behaviour of a decorator to define another class. If you're unfamiliar with this behaviour and reading a codebase using it, it can be a real wild goose chase to find out where the serializer classes actually come from.
The other benefit of your serializer work is the validation of the input data. My bias is to use the type checker to do more validation than they do right now (e.g. an integer between 1-100 or an enum that gives you 5 options).
Okay, so how do you want to define these constraints? They are not expressable in Python's type system. I'm not interested in building a meta-language that uses annotations to express these constraints, as there are already other libraries that do that better (e.g. voluptuous). If you want more specific constraints than can be expressed in Python's type system, you'll need to define the serializer fields (which can handle such constraints) explicitly.
I suppose you want to specify these constraints as arguments to the decorator, which in turn will explicitly define the serializer fields. Then my question is, what's the advantage compared to the proposal in #31?
Btw, enums are actually a Python type, so we should be able to support them natively. I've opened #34 for that.
I like what is proposed in both #31 and #32. What I'm thinking is to push limits until we have everything in the field metadata. I haven't looked into your implementation in detail to make an informed suggestion. So take what I say with a grain of salt.
Agreed that creating hidden classes inside decorators isn't a great practice. But we have large installed base of django users, who are familiar with tooling (django migrations and possibly a lot of other tooling) and a rich ecosystem of type checkers (mypy, pyre-check). So bridging the two worlds will require some sort of magic.
Okay, so how do you want to define these constraints?
My inspiration for this comes from F-star, where I came across the concept of dependent/refinement types. It's true that many ML inspired languages don't have a type system and static checkers which enforce it well.
https://rise4fun.com/fstar/tutorial (section on "simple refinement types")
Here is what I came across in my search for similar concepts in python:
https://github.com/antonagestam/phantom-types/blob/main/phantom/interval.py
Also, I wasn't aware of the concept of serializers described here:
https://www.django-rest-framework.org/api-guide/serializers/
I'm looking to generate:
class Person(models.Model):
name = models.CharField()
email = models.CharField()
...
using a decorator. Not a serializer. But the concepts seem to be close enough that I might be able to reuse some of your work.
I like what is proposed in both #31 and #32.
Oops, I meant #31 and #34.
Here is what I came across in my search for similar concepts in python:
https://github.com/antonagestam/phantom-types/blob/main/phantom/interval.py
That's exactly what I meant with the meta-language that I don't want to invent. It's a valid approach, for sure, but just out-of-scope of this project. Most libraries taking this approach already have validation built in. If someone specifically needs to integrate them with DRF serializers, the glue code for that can be maintained as a separate 3rd party package. I'd even consider PR's to make this more easily extensible for such glue code, but a full implementation is out-of-scope here.
Given that, and that I'm not conviced generating serializers (instead of #31) is a good idea, and
I'm looking to generate [Django models] using a decorator. Not a serializer.
I'll close this issue now, as I don't think there's anything actionable here.
Agreed!
On Sun, Dec 13, 2020, 2:35 AM Oxan van Leeuwen notifications@github.com wrote:
Closed #33 https://github.com/oxan/djangorestframework-dataclasses/issues/33.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oxan/djangorestframework-dataclasses/issues/33#event-4105878521, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFA2A2TK54YODCB4IYNOW3SUSKHLANCNFSM4UW6HZKQ .
https://github.com/adsharma/fquery/pull/2 implements this idea. A few things changed in the last few months:
https://github.com/microsoft/pyright/blob/master/specs/dataclass_transforms.md
It would be nice to write:
and have this part auto generated:
would you consider such a pull request?