jcrist / msgspec

A fast serialization and validation library, with builtin support for JSON, MessagePack, YAML, and TOML
https://jcristharif.com/msgspec/
BSD 3-Clause "New" or "Revised" License
2.01k stars 59 forks source link

Automatically support `functools.cached_property` without requiring setting `dict=True` #623

Open jcrist opened 6 months ago

jcrist commented 6 months ago

Description

Since msgspec.Struct types are effectively slotted classes, using functools.cached_property with structs requires the user to also set dict=True:

import functools
import msgspec

class RightTriangle(msgspec.Struct, dict=True):    # <- dict=True required for cached_property
    a: float
    b: float
    @functools.cached_property
    def c(self):
        return (self.a ** 2 + self.b ** 2) ** 0.5

This is because the cpython implementation of cached_property requires a __dict__ exist on the instance. The error raised if you fail to set dict=True happens on property access, and since it's from functools not msgspec it doesn't indicate how to resolve the issue:

TypeError: No '__dict__' attribute on 'RightTriangle' instance to cache 'c' property.

As such, users have to read the docs (or ask) to know how to mix msgspec.Struct and functools.cached_property together. It would be nice to improve this situation. A few options:

  1. Automatically set dict=True if the class includes any cached_property attributes. This lets things "just work" for most users.
  2. Warn if a class includes cached_property attributes but doesn't set dict=True. This is like a less automagic form of 1.
  3. Rewrite cached_property attributes on msgspec.Struct types if dict=False to use an alternative slots-based mechanism, similar to what attrs added here.

Of these options I'm leaning towards 1 or maybe 3.

jcrist commented 6 months ago

Upon further investigation, I'm now leaning more towards 3.

The main downsides are:

provinzkraut commented 6 months ago
  • Some magic. When used on a Struct type, the cached_property implementation itself won't ever actually run, it's just a decorator flag to tell the Struct implementation to treat it as a cached property using our own implementation. Since attrs is already doing the same, I feel better following their lead.

What about not using functools.cached_property, but instead provide e.g. msgspec.cached_property, which would work the same way you described, but would make it more explicit that this is not the same as the functools one? That would eliminate the magic aspect of it while providing the same functionality.