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

feat: add new show options for docstrings #56

Merged
merged 5 commits into from
Feb 3, 2023

Conversation

thatlittleboy
Copy link
Contributor

@thatlittleboy thatlittleboy commented Jan 29, 2023

Related to mkdocstrings/mkdocstrings#466.

Changes

List of new options (view docstring in git diff for more details):

  • show_docstring_description
  • show_docstring_attributes
  • show_docstring_parameters
  • show_docstring_other_parameters
  • show_docstring_raises
  • show_docstring_warns
  • show_docstring_yields
  • show_docstring_receives
  • show_docstring_returns
  • show_docstring_examples

In total, 10 new options, covering all the docstring types.

Details:

  • My approach was to start with the sections mentioned inside material/_base/docstring.html, but combined section.kind=="admonition" and section.kind=="text" under the same show_docstring_description option. Just felt strange to separate them as I can't imagine a scenario I want one but not the other, but let me know if you prefer them separated.
  • Open to any and all suggestions for improving the option names.
  • I didn't add a "show" for class attributes / class properties, not quite sure if it was in the scope of the original issue or not.
  • I didn't add support for the other themes (mkdocs, rtd).

See more comments in review.

@Archmonger
Copy link

Archmonger commented Feb 2, 2023

Option names are definitely a bit too long right now

I would suggest:

  • show_description
  • show_attributes
  • show_parameters
  • show_other_parameters (note: what is this??)
  • show_raises
  • show_warns
  • show_yields
  • show_receives
  • show_returns
  • show_examples
  • show_title

For completeness, the existing two are

  • show_root_heading
  • show_source

@thatlittleboy
Copy link
Contributor Author

@Archmonger yea, I agree in spirit that the option names are too long, as proposed. And get where you are coming from. On the other hand, my thinking was that I'ld rather be more explicit in the option names than to keep it short & ambiguous, possibly clashing with other options / confusing users.

Options are specified once, in mkdocs.yml or in the autodocs, the length of the option name is not that big a deal IMO.

TBH, I have a bigger gripe with the fact that, with the introduction of this many new options, how do we properly document and illustrate to the user what the effect of all these options are. And not let them feel overwhelmed by all the options.

ps. on Other parameters, see here: https://mkdocstrings.github.io/griffe/docstrings/#other-parameters

@pawamoy
Copy link
Member

pawamoy commented Feb 3, 2023

Unless we start nesting options (which I wouldn't like to start without a clear spec), I agree that the proposed option names are a bit too long. Since docstrings only have sections, we can drop that from the names:

  • show_docstring_description
  • show_docstring_attributes
  • show_docstring_parameters
  • show_docstring_other_parameters
  • show_docstring_raises
  • show_docstring_warns
  • show_docstring_yields
  • show_docstring_receives
  • show_docstring_returns
  • show_docstring_examples

A few questions/remarks:

  • I wonder if show_docstring_body or show_doctring_text would better illustrate the fact that it would render all the text parts of the docstring, not the just the first block of text (which description makes me think of)
  • show_function_title looks like an outlier here. We should either provide this option for titles (which are headings in HTML terms) of all kind of objects, or for none of them. The former sounds like a (non-existing yet) show_object_heading option, specialized for each object kind (show_class_heading, show_function_heading, etc.). I'm not sure providing these options is a good idea as it is right now: not rendering the headings makes it impossible to cross-ref the objects, and they won't appear in the objects inventory or the ToC either. I'm open to discussing this in more details, but it will need to happen in another issue / PR 🙂

Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work, thanks a lot!

@pawamoy
Copy link
Member

pawamoy commented Feb 3, 2023

TBH, I have a bigger gripe with the fact that, with the introduction of this many new options, how do we properly document and illustrate to the user what the effect of all these options are. And not let them feel overwhelmed by all the options.

I'm planning on improving docs. Each option would be properly documented, with visual examples, etc., instead of just rendered from the docstring (this was a convenient way until now, but we can do better). Good point.

Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK LGTM! Thanks a lot!

I just had an idea to improve perfs of the docstring.html template but we can do that in another PR.

@Archmonger
Copy link

Archmonger commented Mar 18, 2023

I just had an idea to improve perfs of the docstring.html template but we can do that in another PR.

Pinging to remind that an issue was never created for this. Just making sure it doesn't get forgotten!

@Archmonger
Copy link

Archmonger commented Oct 16, 2024

Sorry to bump on this merged PR - But I can't figure out how to get the following section to be hidden

image

I was under the assumption that show_docstring_attributes would hide either that chunk of HTML or the one above it. But it doesn't appear to do either.

@pawamoy
Copy link
Member

pawamoy commented Oct 16, 2024

That's not a docstring section, it's a sequence of actual attribute objects being rendered. Try removing their docstring, or filtering them out if you don't want them to appear in the docs.

@Archmonger
Copy link

But isn't the Attributes: section above it the docstring section? I would have assumed show_docstring_attributes would at least hide that segment.

On a tangent, I know my use case is niche but there are certain segments in my docs where I'd prefer only keeping the "concatenated" version contained within Attributes: and not the actual attribute objects.

@pawamoy
Copy link
Member

pawamoy commented Oct 17, 2024

Sorry, I misread. Yes, the Attributes paragraph above your red rectangle seems to be generated from an Attributes section. Can you open a new issue?

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

Successfully merging this pull request may close these issues.

3 participants