Opened 2 years ago

Closed 8 weeks ago

#21042 closed Bug (fixed)

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

Reported by: benoitbryon Owned by: nobody
Component: Database layer (models, ORM) Version: 1.5
Severity: Normal Keywords:
Cc: bmispelon@…, jm.barbier+djangocode@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


I was reading code to understand why Sphinx's autodoc fails to handle some model attributes such as FileField. See 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, raising AttributeError is a feature.

But as mentioned in, returning self could be a solution to get autodoc work (but isn't it a hack?).

Notes and references:

Change History (7)

comment:1 Changed 2 years ago by benoitbryon

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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.

comment:2 Changed 2 years ago by bmispelon

  • Cc bmispelon@… added
  • Triage Stage changed from Unreviewed to Accepted


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.


comment:3 Changed 2 years ago by bmispelon

For reference, I found a reference for the return self in python's source code:

comment:4 Changed 2 years ago by jm.barbier+djangocode@…

  • Cc jm.barbier+djangocode@… added


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


comment:5 Changed 8 weeks ago by timgraham

  • Has patch set
  • Summary changed from Inconsistent __get__() implementation: SingleRelatedObjectDescriptor VS FileDescriptor to Sphinx's autodoc fails to handle FileField due to FileDescriptor implementation

comment:6 Changed 8 weeks ago by charettes

  • Triage Stage changed from Accepted to Ready for checkin

comment:7 Changed 8 weeks ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

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).

Note: See TracTickets for help on using tickets.
Back to Top