agateblue / lifter

A generic query engine, inspired by Django ORM
ISC License
449 stars 16 forks source link

Use of __getattr__ in QuerySet #14

Open Mec-iS opened 8 years ago

Mec-iS commented 8 years ago

From a Python3 perspective, is it not wrong to use __getattr__ here instead of __getattribute__? Is there any design decision involved?

agateblue commented 8 years ago

The fact is I was not aware of the existence of __getattribute__. As the difference seems to be __getattr__ is called when no attribute was found, while __getattribute__ is always called (both and Python2 and Python3), I think that __getattr__ is the correct method to call here: we first check on the manager itself, then we proxy to the queryset. However, I'm possibly missing something here.

Still from that perspective, however, the try block is superfluous: if __getattr__ is called, it means no attribute has been found on the manager itself.

Mec-iS commented 8 years ago

Then, I think the proper way of doing it is:

return getattr(super(Manager, self), attr)

because is the standard way of accessing an attribute in an object. getattr actually implement the Python mechanism for attributes' retrieval that involves also __getattr__ and __getattribute__ use.