Allow abstract subclasses of ModelSchema #63
Conversation
… subclasses that cannot be instantiated and thus they don't have to undergo the strict checks that regular ModelSchemata need to fulfill. Closes !62
mikulas-mrva
force-pushed
the
config-abstract-model
branch
from
820a991
to
3fd4198
Compare
|
Thanks @mikulas-mrva. I'll need to fix some issue in GH workflow and try to review this on the weekend. |
|
@jordaneremieff Any way I can help with the workflow issues? |
|
@mikulas-mrva hey, sorry haven't had a chance to come back to this (that time of the year...). Sure if you want to open another PR that modifies the workflows file/passes then I can merge that and run it for this. It looks like one of more of the Python versions specified for the tests is no longer available. |
|
@jordaneremieff I have changed |
There was a problem hiding this comment.
Thanks @mikulas-mrva. I've added my review comments with some requested changes.
| @@ -40,6 +40,33 @@ class UserSchema(ModelSchema): | |||
|
|
|||
| Once defined, the `UserSchema` can be used to perform various functions on the underlying Django model object, such as generating JSON schemas or exporting serialized instance data. | |||
|
|
|||
| ### Custom subclasses | |||
There was a problem hiding this comment.
Changes:
- Make this heading
### Abstract schema models - Place as the next heading after
Customizing the schema
| @@ -40,6 +40,33 @@ class UserSchema(ModelSchema): | |||
|
|
|||
| Once defined, the `UserSchema` can be used to perform various functions on the underlying Django model object, such as generating JSON schemas or exporting serialized instance data. | |||
|
|
|||
| ### Custom subclasses | |||
|
|
|||
| Abstract subclasses can be defined to implement methods shared over for a number of ModelSchemata, note that they cannot be instantiated by themselves. | |||
| from djantic import ModelSchema | ||
| from myapp.models import User | ||
|
|
||
| class BaseModelSchema(ModelSchema): |
There was a problem hiding this comment.
Can you make more obvious that this is an example and what the use-case is? This may be misinterpreted of how to define all abstract schema. For example, the model: Optional[Model] = None isn't required, but not clear.
| model = User | ||
| include = ["id"] | ||
|
|
||
| schema = InheritedSchema(id=1) |
| @@ -37,6 +37,23 @@ class Config: | |||
| include = ["id"] | |||
| exclude = ["first_name"] | |||
|
|
|||
| class AbstractModelSchema(ModelSchema): | |||
| class Config: | |||
| abstract = True | |||
There was a problem hiding this comment.
What is expected to happen when model, include, or exclude are set on the config? Need coverage for these cases.
There was a problem hiding this comment.
This is a very good point, I'll ping you once I have a minute to look into it.
In my opinion none of those should be present on an abstract model, as its main raison d'etre is to reduce boilerplate and allow proper typing in codebases where multiple ModelSchemata are used in a similar customised manner. Do you have any thoughts on the matter as the repo/package owner?
There was a problem hiding this comment.
I guess it would be best if we started out with some warnings (raise config error) if someone tries to include these on the abstract model to avoid confusion, then if someone later presented a valid use-case to support any of these then maybe could do then if it made sense.
Adding the option to implement custom methods in abstract ModelSchema subclasses that cannot be instantiated and thus they don't have to undergo the strict checks that regular ModelSchemata need to fulfill.
Closes !62