Work in progress: Refactor #232
Conversation
Here we are more careful about which caller's locals we use to
resolve forward type references. We want the callers locals
at decoration-time — not at decorator-construction time.
Consider:
```py
frozen_dataclass = marshmallow_dataclass.dataclass(frozen=True)
def f():
@custom_dataclass
class A:
b: "B"
@custom_dataclass
class B:
x: int
```
The locals we want in this case are the one from where the
custom_dataclass decorator is called, not from where
marshmallow_dataclass.dataclass is called.
When class_schema is called, it doesn't need the caller's whole stack frame. What it really wants is a `localns` to pass to `typing.get_type_hints` to be used to resolve type references. Here we add the ability to pass an explicit `localns` parameter to `class_schema`. We also add the ability to pass an explicit `globalns`, because ... might as well — it might come in useful. (Since we need these only to pass to `get_type_hints`, we might as well match `get_type_hints` API as closely as possible.)
Manually merge work in PR lovasoa#172 by @onursatici to support generic dataclasses Refactor _field_for_schema to reduce complexity.
Handle default generic params values in _field_for_builtin_collection_type.
This implement the suggestions made in lovasoa#51. See lovasoa#51 (comment)
Class_schema does not currently support generating schemas for classes which have generic base classes. Typing.get_type_hints doesn't properly dereference the generic parameters. (I think it can be done, but I don't think it's simple.)
This fixes thread-safety issues in lazy_class_attribute. Here we also specialize to our use case, thus cleaning up the type annotations
FIXME: may should warn instead of raising exception
Retain relaxed checking of tests for now
This allows us to cleanly eliminate the thread-local context stack.
|
@dairiki Any progress on this? Is there anything others can do to help? |
As you've probably noted. I stalled on this and haven't looked at it in some time. As I was working on this, that I kept bumping up against issues with correctly supporting the Since this PR was turning into a near-complete rewrite, and since the So, I guess, some community (and maintainer) guidance as to the appetite for: |
|
FWIW, in my previous work as a user ofmarshmallow-dataclass, we never used the @dataclass
class City:
name: Optional[str]
buildings: List[Building] = field(default_factory=list)
CitySchema = marshmallow_dataclass.class_schema(City)For the cost of 1 extra LOC, you get clearer code that separates dataclass and Schema, and makes type checkers happy. |
@sloria Me too. And I agree. However, based on tickets opened here and general discussion, there are quite a few who do use the magic decorator. |
|
Personally I'm okay with the removal of the decorator but certainly not with the forced use of That being said, even pydantic supports the dataclass decorator, but with limitations. Which I think is a good compromise? |
|
Regarding forward references I've done something similar in the past: https://github.com/mvanderlee/starmallow/blob/master/starmallow/utils.py#L383 and pydantic does this as well. |
I've fallen down a rabbit hole and have started work on a significant refactor of the code-base.
This refactor pulls in work from PR #172 as well as PR #214.
It is a work in progress. It's not done, however, it should theoretically be in a working state.
Comments are welcome.
Summary
.Schemaattribute.)pd.Timestamp), we recursively try to convert into a dataclass which mutates the constructor throughout the codebase #51.class_schema. This includes raising more specific custom exceptions when there are problems with the input toclass_schema.Issues Addressed
It fixes #230, #198, #229, #193, and #51.
It can probably address #143.
It could possibly address #146, and #56.
I haven't exhaustively checked for issues that may be addressed — there are probably more.