Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ntouran opened this issue Mar 1, 2023 · 4 comments · Fixed by #33
Closed

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

ntouran opened this issue Mar 1, 2023 · 4 comments · Fixed by #33
Assignees
Labels

Comments

@ntouran
Copy link

ntouran commented Mar 1, 2023

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
Copy link
Collaborator

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

@ntouran
Copy link
Author

ntouran commented Mar 1, 2023

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
Copy link
Collaborator

timobrembeck commented Mar 1, 2023

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants