Post-review of transition to `attrs`

inducer / pytato

Lazily evaluated arrays in Python

Other

11 stars 16 forks source link

Open inducer opened 2 years ago

inducer commented 2 years ago

After merging #365, I noticed a few things I wish I'd flagged, see https://github.com/inducer/pytato/pull/365#pullrequestreview-1147957251. They're not a huge deal, but I think we should address them to avoid setting traps for our future selves.

kaushikcfd commented 2 years ago

matthiasdiener commented 4 months ago

Out of curiosity, should we now consider moving back to dataclasses (which would imply Python 3.10+)?

inducer commented 4 months ago

In general, I'm in favor of that. Good reasons to go back:

uniformity? Everywhere else in the stack uses dataclasses.
https://docs.python.org/3/library/typing.html#typing.dataclass_transform would allow us to define a single class decorator for array-likes, vs. the inconsistent mess that we have currently.

Obstacles:

We're using cache_hash=True in attrs, for which dataclasses has no equivalent. We would need to generate our own code for __hash__. Could be worse, I think.