cattrs icon indicating copy to clipboard operation
cattrs copied to clipboard

Add generic `omit_if` to support more than just `omit_if_default`

Open redruin1 opened this issue 6 months ago • 6 comments

This PR adds an omit_if parameter to cattrs unstructure generators (as described here), which is similar in function to omit_if_default but evaluates a callable at unstructure invoke in order to determine whether an attribute should be omitted:

@define
class Example:
    a: int
    b: int | None
    c: int
    d: int = 123
    e: set = field(factory=set)
    f: int = field()

    @f.default
    def f_default(self) -> int:
        return self.d

    overridden: None = None

converter = Converter()

def default_or_none(instance: AttrsInstance, attribute: Attribute, value: Any) -> bool:
    if value is None:
        return True
    if isinstance(attribute.default, Factory):
        if attribute.default.takes_self:
            return value == attribute.default.factory(instance)
        return value == attribute.default.factory()
    return value == attribute.default    

converter.register_unstructure_hook(
    Example,
    make_dict_unstructure_fn(
        Example,
        converter,
        _cattrs_omit_if=default_or_none,
        c=override(omit_if=lambda inst, attr, value: value < 0),
        overridden=override(omit_if=False),
    ),
)

assert converter.unstructure(Example(a=100, b=None, c=-100, f=123)) == {"a": 100, "overridden": None}

This allows users to control how the unstructure function should behave at runtime (beyond the singular omit_if_default), wheras currently users can only robustly choose how/when to omit fields when the function is first generated.

omit_if is technically a superset of omit_if_default, hence their similar names. However, this implementation leaves omit_if_default in function signatures for 3 reasons:

  1. Backwards compatibility
  2. Performance. An apples-to-apples comparison using a lambda instead of generating in-situ is about twice as slow, at least according to the minimal benchmarks I wrote and performed. This could maybe be improved, but I don't see it ever being faster than embedding the checks directly into the function.
  3. attrs doesn't currently have a convenient get_default(instance, attribute) equivalent, which means they must write their own function if somebody just wants the old omit_if_default behavior with the new omit_if parameter - which will lead to dozens of slightly different implementations and (of course) bugs.

If both omit_if_default and omit_if are defined, then omit_if_default takes priority. However, I can also see a world where defining both simultaneously is forbidden, as which should take precedent is not intrinsically apparent.

This is a proof of concept PR, it is not currently complete. I am interested in completing it (ensuring coverage/writing docs/etc.) but I want the go-ahead before I invest serious amounts of time into it.

redruin1 avatar Apr 02 '25 08:04 redruin1