python-attrs / attrs

Python Classes Without Boilerplate
https://www.attrs.org/
MIT License
5.3k stars 374 forks source link

Define __getattribute__() instead of __getattr__() on slotted classes with cached properties #1291

Closed dlax closed 4 months ago

dlax commented 5 months ago

Summary

Fix https://github.com/python-attrs/attrs/issues/1288

Method __getattribute__() is documented as the "way to to actually get total control over attribute access" so we change the implementation of slotted classes with cached properties by defining a __getattribute__() method instead of __getattr__() previously.

Just changing that preserves the current behaviour, according to the test suite, but also makes sub-classing work better, e.g. when the subclass is not an attr-class and also defines a custom __getattr__() as evidenced in added test from #1288.

In tests, we replace most custom __getattr__() definitions by equivalent __getattribute__() ones, except in regression tests where __getattr__() is explicitly involved. Also, in test_slots_with_multiple_cached_property_subclasses_works(), we replace the if hasattr(super(), "__getattr__"): by a try:/except AttributeError: as using hasattr(..., "__getattribute__") would be meaningless since __getattribute__() is always defined.

Pull Request Check List

codspeed-hq[bot] commented 4 months ago

CodSpeed Performance Report

Merging #1291 will not alter performance

Comparing dlax:issue-1288 (1d845e3) with main (edcaf04)

Summary

✅ 12 untouched benchmarks

hynek commented 4 months ago

Closing due to https://github.com/python-attrs/attrs/issues/1288#issuecomment-2227820600 – thank you very much for the work though! Sometimes it takes an experiment to find out.