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

[FEATURE] Overwrite encoder via to_dict? #451

Closed
powellnorma opened this issue Jul 29, 2023 · 5 comments
Closed

[FEATURE] Overwrite encoder via to_dict? #451

powellnorma opened this issue Jul 29, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@powellnorma
Copy link

powellnorma commented Jul 29, 2023

Description

It can be useful to supply a custom encoder function (e.g. for #176 (comment)). Being able to do that via overwriting to_dict seems intuitive. However it looks like to_dict is not called for nested classes?

@dataclass
class A(DataClassJsonMixin):
    _x: int
                               
    def to_dict(self, encode_json):
        r = super().to_dict(encode_json)
        r['x'] = r.pop('_x')
        return r
                               
@dataclass
class B(DataClassJsonMixin):
    a: A
                               
a = A(1)
b = B(a)
print(a.to_json())  # {"x": 1}
print(b.to_json())  # {"a": {"_x": 1}}

Possible solution

Check if object is a DataClassJsonMixin, and if so use to_dict while encoding

Alternatives

  • One can already do the following:

    import dataclasses_json.cfg
    dataclasses_json.cfg.global_config.encoders[A] = lambda x: x.to_dict()

    However current to_dict nested behavior may be unintuitive for new users + this is more verbose.

    Also this does not generalize to related type annotations, as the key used in encoders has to match exactly the type annotation. E.g. if a class C has all_a: list[A], we would have to add:

    dataclasses_json.cfg.global_config.encoders[list[A]] = lambda l: [x.to_dict() for x in l]
  • One could in this case also use field(metadata=config(..))
    But e.g. when wanting to add additional attributes (as in Support properties #176) this is not enough

Context

No response

@powellnorma powellnorma added the enhancement New feature or request label Jul 29, 2023
@george-zubrienko
Copy link
Collaborator

george-zubrienko commented Jul 30, 2023

@powellnorma We'll start v1 API next week - please take a look at #442, it should solve your problem. Re implementing something in v0, I'm not sure its worth the time.

@powellnorma
Copy link
Author

powellnorma commented Jul 30, 2023

Hm ok, iiuc in v1 there will be no classes we can inherit for a to_dict method?
So in v1 in order to overwrite the encoding behavior of a class, we would again do this?

dataclasses_json.cfg.global_config.encoders[DataPerson] = encode_data_person

Or will it be possible to define the encoding behavior in the class itself (without setting global_config.encoders explicitly?) I personally think that would make sense, e.g. with pickle one can define __setstate__ and __getstate__.

Also, will one only need to set

dataclasses_json.cfg.global_config.encoders[DataPerson] = encode_data_person

or also

dataclasses_json.cfg.global_config.encoders[list[DataPerson]] = ..

etc?

Thank you!

@george-zubrienko
Copy link
Collaborator

Basically you will need to define a serializer for the type in question and register it with smth like dcj.register(JsonSerializer[MyType]). Then if you have a = MyType(), you simply call to_dict(a) or to_json(a). There are more nuanced cases as well, noted in the issue.
But in general, mutating user classes goes away in v1 and serDe behaviour becomes an addon you can modify any way you like by extending base serializer functionality.

@powellnorma
Copy link
Author

Thank you. Would it make sense to let the base serializer check for a special method like to_dict or sdj_to_dict?
Or perhaps could we let the user define a custom default serializer?

@george-zubrienko
Copy link
Collaborator

Or perhaps could we let the user define a custom default serializer?

This, most likely. I'll make sure to ping you on related PRs and readme updates when time comes :)
Also thanks for raising the issue, it is important for us to understand the use cases that are problematic in v0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants