Sphinx's autodoc fails to handle FileField due to FileDescriptor implementation

[Benoît Bryon]

I was reading code to understand why Sphinx's autodoc fails to handle some model attributes such as FileField. See ​https://bitbucket.org/birkenfeld/sphinx/issue/1254/autodoc-fails-to-handle-descriptors-with for details...

... then I wondered why SingleRelatedObjectDescriptor.get() returns self if instance is None whereas FileDescriptor.get() raises AttributeError().

I guess they should have the same behaviour. But I'm not sure since I do not know the reason why they differ...

As I understand ​http://docs.python.org/2/reference/datamodel.html#object.__get__, raising AttributeError is a feature.

But as mentioned in ​https://github.com/deschler/django-modeltranslation/pull/131, returning self could be a solution to get autodoc work (but isn't it a hack?).

Notes and references:

• SingleRelatedObjectDescriptor's implementation: ​https://github.com/django/django/blob/c7d0ff0cad24dbf444f2e4b534377c3352c7aa98/django/db/models/fields/related.py#L182
• FileDescriptor's implementation: ​https://github.com/django/django/blob/31e6d58d46894ca35080b4eab7967e4c6aae82d4/django/db/models/fields/files.py#L157
• Creator (and thus many custom fields, including third-party such as django-uuidfield) is also affected: ​https://github.com/django/django/blob/03d9566e0df45c72bffa99fe244a92f0279da56f/django/db/models/fields/subclassing.py#L33

[Benoît Bryon]

Does #11353 refers to the same problem?

After this modification I can use autodoc however I am not sure if this is not breaking something else or only hiding the problem.

[Baptiste Mispelon]

Hi,

I did some digging, and it appears that the return self line in SingleRelatedObjectDescriptor.__get__ was added in commit 5ef0c03ae9aca99289737ba6d88a371ad95cf432 as a fix for #8248.

As for FileDescriptor.__get__, as far as I can tell it was introduced in 7899568e01fc9c68afe995fa71de915dd9fcdd76 and the initial implementation already raised an AttributeError.

So all in all, it seems that the inconsitency was not intended and fixing it with a return self might be the correct way to go (as the commit message for 5ef0c03ae9aca99289737ba6d88a371ad95cf432 mentions, this is supposedly the way that the builtin property does it).

I think it would also be worth it to check if there are other descriptor in django's code that have the same issue and we could also add a comment to clarify the intent of that particular line.

Thanks.

[Baptiste Mispelon]

For reference, I found a reference for the return self in python's source code: ​http://hg.python.org/cpython/file/1d88d04aade2/Objects/descrobject.c#l1243

[jm.barbier+djangocode@…]

Hello

changing the ̀AttributeError to return self effectively solves Sphinx's autodoc problem.

Running the entire django 1.5.5 test suite with this modification does not raise any problem except of course in modeltests.files.tests.FileStorageTests (the self.assertRaises(AttributeError, lambda: Storage.normal) assertion on line 25)..

It would be great to have this change in 1.6

Thanks

[Tim Graham]

​PR

[Tim Graham <timograham@…>]

In 9f6b7047:

Fixed #21042 -- Allowed accessing FileDescriptor on the model class.

This is consistent with ability to reference other descriptors
on the model class (5ef0c03ae9aca99289737ba6d88a371ad95cf432).