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 request: @dataclass_pyrsistent for use with dataclasses #266

Open
MatrixManAtYrService opened this issue Dec 8, 2022 · 2 comments

Comments

@MatrixManAtYrService
Copy link

MatrixManAtYrService commented Dec 8, 2022

Hi, I just started using pyrsistent, but it's working great so far. Thanks for maintaining it.

Here's some code that I dislike.

@dataclass_json
@dataclass(frozen=True)
class Foo:
    bar: Dict[str, str]

I dislike it because you might think that frozen=True means that you're in immutable-land, but it's actually quite possible to mutate objects of this type:

x = Foo({"a":"a"})
x.bar["b"] = "b"
print(x.to_json()) # {"bar": {"a": "a", "b": "b"}}

You can be disciplined, and have both an immutable dataclass and immutable field types:

@dataclass_json
@dataclass(frozen=True)
class Foo:
    bar: PMap[str, str]

But it's easy to forget/miscommunicate and accidentally add a mutable field. That's not pyrsistent's problem, but maybe pyrsistent is in a good position to provide a solution:

@dataclass_json
@dataclass_pyrsistent
@dataclass(frozen=True)
class Foo:
    bar: Dict[str, str]

The decorator would translate the mutable container type to PMap (or PVector) and call freeze on Dicts (or Lists) upon initialization. If you provide a type that pyrsistent can't translate (like a non-pyrsistent-dataclass) then it could just fail with a nice message.

I've included dataclass_json in this request as an example of a library that:

  • does something similar
  • might appear in context with the proposed decorator

Thanks for considering it!

@tobgu
Copy link
Owner

tobgu commented Dec 29, 2022

Hi!

Tighter integration with dataclasses through a decorator, like dataclass_json does, is a potentially very nice idea. Thanks for bringing it up!

In addition to turning the dicts and lists into their frozen counterparts I would also consider turning the whole class into a PClass (if possible in an elegant way). This could become an alternative way of specifying PClasses that would potentially play better with existing tooling for code completion, type checking, etc. than the current way does.

I'd be happy to discuss this further, review PRs, etc. if you want to work on this!

@MatrixManAtYrService
Copy link
Author

I checked back in on this issue because I ran into something that reminded me of it, so I anticipate it'll keep coming up.

I'd want to spend a bit more time with pyrsistent and immutable data in python in general before attempting such a contribution, so I'm not going to hop to it just yet. But in general--yeah that sounds great.

I may ping you for feedback on a draft PR in the future (though I'd love it if somebody beat me to it because I am not fast).

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

No branches or pull requests

2 participants