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

autodoc: Only add 'alias of' line for aliases having no docstr (#12484) #12498

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

felixblanke
Copy link

Subject: autodoc: Do not add an 'alias of ...' line using the autodata and autoattribute directives for type aliases having docstrings.

Feature or Bugfix

  • Bugfix

(probably bugfix as this is the behavior for the autoclass directive)

Purpose

Currently, if the directive autodata or autoattribute are used on a type alias, the text alias of <type> is added to the end of the documentation text – even if a docstring for the alias is provided.
This is inconsistent with the autoclass directive that does not add such a line if a docstring is provided.

Detail

The 'alias of...' line is added by the method update_content of GenericAliasMixin. This PR adds a check s.t. this method is only called if no docstring is provided by the user.

Relates

autodoc: Do not add an 'alias of ...' line using the `autodata` and
`autoattribute` directives for generic aliases having docstrings.
@felixblanke felixblanke force-pushed the autodoc-add-alias-of-only-if-no-docstr branch from 04b3756 to 50d7791 Compare June 30, 2024 23:18
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

I like this feature because it's annoying to have "alias of [...]" which sometimes is not helpful at all or when it renders incorrectly.

CHANGES.rst Outdated Show resolved Hide resolved
@@ -2152,7 +2152,8 @@ def add_content(self, more_content: StringList | None) -> None:
if not more_content:
more_content = StringList()

self.update_content(more_content)
if not self.get_module_comment(self.objpath[-1]):
Copy link
Member

@picnixz picnixz Jul 20, 2024

Choose a reason for hiding this comment

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

The issue with this path is that it entirely disallows a documenter to subclass update_content. An alternative is to provide a method, say should_update_content which returns a boolean determining whether the directive should update the content or not. By default, this would return not self.get_module_comment(self.objpath[-1]) and subclasses could decide what to do.

EDIT: if this is how the class directive proceeds, you can keep this logic.

Copy link
Author

Choose a reason for hiding this comment

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

This is handled the same way in ClassDocumenter

if self.doc_as_attr and not self.get_variable_comment():

So shall we keep it this way for now?

@AA-Turner AA-Turner added this to the 8.x milestone Jul 23, 2024
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