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

Add support for short signatures in autosummary #13172

Merged
merged 13 commits into from
Jan 29, 2025

Conversation

timhoffm
Copy link
Contributor

@timhoffm timhoffm commented Dec 6, 2024

This introduces a new :signatures: option to .. autosummary::.

Possible values are:

  • "long" (default) - same as the current default
  • "short": format functions and classes as func() if they do not have arguments and func(…) if they do have arguments.
  • "none": same as the already existing :no-signature:

The new functionality here is the "short" option. This allows to distinguish properties and attributes from functions and classes without costing a lot of space for a long signature.

Note 1: Design: I've added the :signatures: option to have a single config value instead of :shortsignatures: alongside :no-signatures:. This is also more extensible in case other signature variants come up in the future. :no-signatures: remains available for backward compatibility, but :signatures: takes precedence if given.

Note 2: Tests: There are currently no tests for the signature formatting. I could add some tests for Autosummary.get_items, but I don't understand how to set up everything in a test so that I can exercise that function. Please give a hint how to do this if tests are desired.

Closes #13101.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
ewfian Fan
get_items() already formats the signature. Handling :nosignatures: option in the saves the signature computation cost if the signature is not wanted. Also, this prepares for more complex signature options.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This is in preparation of extending singature formating.
@timhoffm timhoffm force-pushed the autosummary-short-signature branch from b87c5bc to cd95331 Compare December 7, 2024 07:55
This formats functions and classes with (…) if they have
arguments and () if they don't have arguments. This makes
it easier to distinguish callables from attributes and
properties.
@timhoffm timhoffm force-pushed the autosummary-short-signature branch from 5b082a7 to bf01890 Compare January 22, 2025 15:12
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

It would be nice to have tests, but I'm also unfamiliar with autosummary, I'd be starting from scratch. Other than that this looks good!

Please can you update it in light of #13264?

A

@timhoffm
Copy link
Contributor Author

timhoffm commented Jan 24, 2025

#13264 has renamed :nosignatures: to :no-signatures:. Since this PR introduces :signatures: [style] and :signatures: none is equivalent to :no-signatures: should I remove :no-signatures: again and directly recommend :signatures: none as a replacement for :nosignatures:? If people have to move anyway, we can directly steer them to the more general :signatues: [style] and thus get rid of the redundancy with the expiration of :no-signatures:.

@AA-Turner
Copy link
Member

Sure, if it makes more sense to deprecate :nosignatures in the long run then let's do so -- I wasn't sure if that was your goal here!

A

@timhoffm
Copy link
Contributor Author

It wasn't originally, to not force user changes. But since the renaming already does that, we can get to a cleaner state with no additional cost.

# Conflicts:
#	doc/usage/extensions/autosummary.rst
#	sphinx/ext/autosummary/__init__.py
Since we introduced the more general ``:signatures: none` spelling,
we can directly push people in this direction.
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

Needs a CHANGES entry.

A

CHANGES.rst Outdated
Comment on lines 50 to 53
pattern doesn't match any documents, via the new ``toc.glob_not_matching``
warning sub-type.
Patch by Slawek Figiel.
* #9732: Add the new ``autodoc.mocked_object`` warnings sub-type.
* #9732: Add the new ``autodoc.mock_objects`` warnings sub-type.
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strange. That's of course unintended.. I possibly have messed up git when bringing in the remote upstream merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok fixed. I'm struggling with the merge-main-into-branch workflow. I'm much more comfortable with rebases. Sorry for the confusion.

Copy link
Member

Choose a reason for hiding this comment

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

No worries -- I tend to use the merge interface in PyCharm for tricky merges, which simplifies things greatly.

@AA-Turner AA-Turner added this to the 8.2.0 milestone Jan 29, 2025
@AA-Turner AA-Turner merged commit 5871ce2 into sphinx-doc:master Jan 29, 2025
22 checks passed
@AA-Turner
Copy link
Member

Thank you @timhoffm!

A

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Short signatures for autosummary
2 participants