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

_decode_dataclass ignores InitVars because of use of fields #472

Open
NiroHaim opened this issue Aug 13, 2023 · 5 comments · May be fixed by #495
Open

_decode_dataclass ignores InitVars because of use of fields #472

NiroHaim opened this issue Aug 13, 2023 · 5 comments · May be fixed by #495
Assignees
Labels
bug Something isn't working

Comments

@NiroHaim
Copy link

Description

My usecase requires me to work with initvars and override the to_dict method slightly.
Each time I convert my obj to dict, I need to replace a protected attr with its initvar.
The problem is that when I try to create an obj from said dict, I fail because the initvar attr that exists in the class is filtered out.
The initvar is filtered out because in the _decode_dataclass function, to create the init_kwargs dict, the fields function is used, which ignores initvars.

Code snippet that reproduces the issue

import dataclasses

from dataclasses_json import DataClassJsonMixin


@dataclasses.dataclass
class A(DataClassJsonMixin):
    a_init: dataclasses.InitVar[int]
    _a: int = None

    def __post_init__(self, a_init: int):
        self._a = a_init + 1

    @property
    def a(self):
        return self._a

    @a.setter
    def a(self, value):
        self._a = value

    def to_dict(self, *args, **kwargs):
        base_dict = super().to_dict(*args, **kwargs)
        base_dict['a_init'] = base_dict.pop('_a')

        return base_dict


if __name__ == '__main__':
    A.from_dict(A(a_init=1).to_dict())  # raises: {TypeError}TypeError("A.__init__() missing 1 required positional argument: 'a_init'")

Describe the results you expected

as to_dict is overridable and a_init is an passable attr for A.init, I expect a_init to be included in init_kwargs in _decode_dataclass when creating the obj.

Python version you are using

python 3.10

Environment description

dataclasses-json==0.5.14
marshmallow==3.20.1
mypy-extensions==1.0.0
packaging==23.1
typing-inspect==0.9.0
typing_extensions==4.7.1

@NiroHaim NiroHaim added the bug Something isn't working label Aug 13, 2023
@matt035343
Copy link
Collaborator

Seems to be a duplicate of #281

@NiroHaim
Copy link
Author

Oh! seems I missed it
anyways, I wrote a small function that gets all initvars from a dataclass, only did basic testing of course but maybe this could help:

    @classmethod
    def _get_initvar_names(cls) -> list[str]:
        annotations = cls.__annotations__.copy()

        for base in cls.__bases__:
            if hasattr(base, "__annotations__"):
                annotations.update(base.__annotations__)
        return [name for name, typ in annotations.items() if isinstance(typ, dataclasses.InitVar)]

On another note, I saw that the issue remains open from 2021, are there currently any intentions to resolve it?

@matt035343
Copy link
Collaborator

matt035343 commented Aug 15, 2023

Hi @NiroHaim, this repo has been without much activity for a long time, so issues have been piling up. A few new maintainers have been added (including myself), so we will try to reduce the backlog as much as possible. This includes looking into the issue described here. I cannot guarantee when, but we will definitely have a look when time allows.

@matt035343 matt035343 self-assigned this Aug 18, 2023
@matt035343
Copy link
Collaborator

matt035343 commented Aug 18, 2023

When I am looking at the documentation for InitVar, it looks like it is supposed to be passed to the constructor, but it should not be accessible from the outside afterwards, e.g. through fields(). Therefore, I would not expect it being returned from to_dict() or to_json() either. Therefore, A.from_dict(A(a_init=1).to_dict()) would never work if we were to respect this assumption.

@NiroHaim What are you expectations to handling this discrepancy?

Looping in contributors of the original issue @AbdulRahmanAlHamali and @ALiangLiang. Feel free to contribute to the discussion.

@NiroHaim
Copy link
Author

Hey @matt035343 , Thanks for the response
I believe that the correct behaviour would indeed not be to include a_init as a key in the to_dict() result, that would just be odd in the general case. (Also changing the output of to_dict would probably be a pain for some projects to upgrade which you would probably want to avoid)
However, I do think that as the InitVars are valid init attributes that are expected to be received, I expect them not to be filtered out when a user chooses to add them to a dict and use from_dict on said dict.
If you want to limit the usage of from_dict to only receive input from the to_dict function that Is fine, but if the purpose is for them to be seperate functions that also work in tandem, I think you should filter out all InitVars through a method similar to what I suggested above and just make sure that the values are there for the init call inside the from_dict function.

**Do note that adding the option for passing initvars to from_dict would allow fun features such as wrapping from_dict to change the private _a to a_init keyword and making the a prop a counter of how many times this json has been converted to A type by adding 1 to a_init each time.
This is of course a rather dull case, however the option of passing InitVars would be very beneficial by allowing the user more freedom when using and wrapping the class.

@matt035343 matt035343 linked a pull request Oct 15, 2023 that will close this issue
@matt035343 matt035343 linked a pull request Oct 15, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants