sphinx-doc / sphinxcontrib-django

This is a sphinx extension which improves the documentation of Django apps.
https://pypi.org/project/sphinxcontrib-django/
Apache License 2.0
43 stars 25 forks source link

Merging of model field docstrings isn't rendering 2nd order directives #32

Closed ntouran closed 1 year ago

ntouran commented 1 year ago

Hi, I'm trying to use this package in concert with another one that defines other directives (sphinx-needs).

I have a model field that has help text and its own docstring. This packages merges them as expected, but does not allow the directive in the previously-existing docstring to get rendered as a directive.

input:

    description = models.JSONField(
        blank=True,
        null=True,
        help_text="The detailed description of the requirement. Should be concise, singular, and written in the form of a 'shall'.",
    )
    """
    .. impl:: Store description as JSONField for richtext purposes
        :links: R_REQ_RICH_TEXT

    |
    """

Current result:

image

Desired result:

Something more like: image image

timobrembeck commented 1 year ago

@ntouran Thanks for reporting the problem. I think I fixed it in 6869e76e7c6ac9f44415d8093f42df26081eb42d and released a new version 2.1. Could you check whether this fixes the issue for you?

ntouran commented 1 year ago

Hey, very nice, thanks! that absolutely works great for 'regular' directives like .. note:: as you've shown. That works nice and is a great improvement.

There is still an issue with my particular use case, which is related to how some of the directives in sphinx-needs work. Somehow I think the docstring is getting processed twice, which is causing an error in sphinx-needs related to seeing the same directive with the same ID twice.

Sphinx error:
A need with ID I_DOC_IMPL_WOO already exists! This is not allowed. Document api/documents[998]

Maybe the merged docstring that you make is getting left in two places somehow?

Anyway I wouldn't blame you for closing this issue since you did make a good improvement. If there's anything you see that might render the directive in the docstring twice that'd be great to remove.

My input is

    description = models.TextField(
        blank=True, help_text="Description of what this doc type is for"
    )
    """description field.

    .. impl:: Use description for descriptoin.
        :id: I_DOC_IMPL_WOO

        It's a model field. 

    """
timobrembeck commented 1 year ago

Ah, interesting, I wasn't aware of such limitations. My guess would be that the fields is included once in the model's parameter documentation:

Screenshot 2023-03-01 at 22-54-05 Models — integreat-cms 2023 2 2 documentation

and once as individual field:

Screenshot 2023-03-01 at 22-54-18 Models — integreat-cms 2023 2 2 documentation

I'm not sure if there is something I can do on my side, maybe an option to disable the inclusion of docstrings in the parameter documentation of models?

In your case, I think it would be the easiest to make use of the autodoc-skip-member signal by putting this into your conf.py:

from django.db.models.query_utils import DeferredAttribute

def skip_model_field(app, what, name, obj, skip, options):
    return (
        name == "description"
        and isinstance(obj, DeferredAttribute)
        and obj.field.model.__name__ == "YourModelName"
    ) or None

def setup(app):
    app.connect("autodoc-skip-member", skip_model_field)

Interestingly, I just found a small bug in this extension which does prevent this code to work (#35), but I will fix this asap.

timobrembeck commented 1 year ago

I've released version 2.2 now, could you test whether the snippet I provided does the job?