Allow sphinx-autodoc to introspect translated fields #131

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@benoitbryon

I'm using Sphinx autodoc and Sphinx apidoc to generate documentation about code, including my application models. Some of them use django-modeltranslation.

Sphinx introspects the models classes and tries to get documentation (doc attribute) of each class member, including translated fields.

Currently, django-modeltranslation raises a ValueError when Sphinx tries to access fields' doc, and Sphinx autodoc fails.

This pull-request proposes a fix where the ValueError exception is replaced with logging: developers keep getting warned, and Sphinx-autodoc succeeds.

@Natim Natim Logging a warning instead of raising ValueError when trying to access…
… translated fields via class. Allows introspection of field's documentation (Sphinx autodoc).
030496c
@zlorf
Collaborator
  1. Getting __doc__ from non-translated fields succeed?
  2. Does Sphinx try to access anything but __doc__?
@benoitbryon

Short answers:

Getting doc from non-translated fields succeed?

Partly: some are listed in documentation (ForeignKey), some are not (BooleanField). None of them produce the same warnings as translated fields.

Does Sphinx try to access anything but doc?

I'm only sure of "doc". But I guess that other attributes such as "name" or "class" could be used too. The default value is also documented, but I don't know how it is evaluated.

Long answers below...

It's not an error, but a warning

First of all, I have to admit that the sphinx build succeeds (the build ends, but produces warnings). So, the issue is not blocker.

Nevertheless, I feel that it would be great if translated fields supported Sphinx introspection.

More about the problem

sphinx-apidoc doesn't produce errors or warnings. It produces reStructuredText code, such as:

:mod:`models` Module
--------------------

.. automodule:: postbox.channel.models
    :members:
    :undoc-members:
    :show-inheritance:

Warnings are reported during sphinx-build, via sphinx-autodoc extension. Something like:

WARNING: autodoc can't import/find attribute 'myapp.models.MyModel.title', it reported error: "title", please check your spelling and sys.path

The HTML file is produced, i.e. we get HTML documentation for myapp.models.MyModel.
But the "title" attribute isn't documented, I mean it is not listed as an attribute of the model.
=> I'd like the "title" attribute to be listed, even if the related documentation is empty.

Some of the other fields are displayed, some are not. As an example, ForeignKey fields are listed, whereas BooleanField are not. I haven't figured out why fields are displayed or not. Anyway, fields that are not "multilingual" don't produce warnings. Is that a bug or a feature, I don't know right now.

Note: in my specific case, I have a test which checks that the Sphinx build succeeds, i.e. doesn't produce a single warning (sphinx-build warnings often mean broken documentation). So, in my case, this issue breaks the tests :( but I admit that's a personal use-case.

With the patch proposed in this pull-request:

  • sphinx-build doesn't report warnings, my tests pass
  • but the field is still not listed in documentation...

=> The implementation of the pull-request solves only a part of the issue.

About Sphinx autodoc implementation

As you asked, here are more details about Sphinx implementation:

Right now, I don't know more about Sphinx implementation.

@zlorf
Collaborator

I think this issue should be resolved as in django itself: https://github.com/django/django/blob/master/django/db/models/fields/related.py#L260 and https://github.com/django/django/blob/master/django/db/models/fields/related.py#L283.
With such a code sphinx would get descriptor instance and could get it's __doc__.

Question to Dirk: is this exception in https://github.com/deschler/django-modeltranslation/blob/master/modeltranslation/fields.py#L156 important for some reason?

@zlorf
Collaborator

@deschler: Please take a look at branch descriptor_introspection (dbc114f), i have resolved issue the way the Django did.
However, some exception raising was removed. What do you think?

@deschler
Owner

@zlorf: The exception isn't really important, nor do we rely on it anywhere, it's just a common pattern to raise a ValueError in this case. Since Django returns self, i'd say it's fine. Feel free to merge the branch.

I'm just wondering how many other apps are affected of the same problem. Try googling for AttributeError AND "can only be accessed via an instance", that brings up quite some hits.

@zlorf
Collaborator

Resolved in dbc114f.

@zlorf zlorf closed this Jan 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment