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

Make fallback for changed get_signature_prefix() #9833

Merged
merged 3 commits into from Nov 10, 2021

Conversation

@jakobandersen
Copy link
Contributor

@jakobandersen jakobandersen commented Nov 9, 2021

Feature or Bugfix

  • Bugfix

Purpose

In #9672 the signature of get_signature_prefix() in the Python domain was changed to return a list of docutils nodes instead of string. Based on #9832 it looks like some extensions (https://pypi.org/project/autodoc-pydantic/) indeed overwrites this method.
This PR adds a fallback check for str and warns about the changed interface.

Detail

This is quick and dirty PR to get started. I guess it should use one of the deprecation facilities, but I'm not exactly sure.

Relates

Fixes #9832

@@ -495,7 +495,15 @@ def handle_signature(self, sig: str, signode: desc_signature) -> Tuple[str, str]

sig_prefix = self.get_signature_prefix(sig)
if sig_prefix:
signode += addnodes.desc_annotation(str(sig_prefix), '', *sig_prefix)
if type(sig_prefix) is str:
logger.warning(
Copy link
Member

@tk0miya tk0miya Nov 9, 2021

Choose a reason for hiding this comment

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

Could you use warnings.warn() instead? It can describe what version this fallback will be dropped.

Copy link
Contributor Author

@jakobandersen jakobandersen Nov 9, 2021

Choose a reason for hiding this comment

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

Sure. Removed in v5 or v6?
Is there a way to automatically attach the location that triggered this? (the location=signode that logger.warning() accepts)?

Copy link
Member

@tk0miya tk0miya Nov 10, 2021

Choose a reason for hiding this comment

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

Our deprecation policy is "the deprecated feature will work during 2 MAJOR releases at least". So it will be removed in v6.0.
https://www.sphinx-doc.org/en/master/internals/release-process.html#deprecation-policy

Is there a way to automatically attach the location that triggered this?

No way to do that. It would be nice if we can improve it in the future.

@tk0miya tk0miya merged commit 5999cdb into sphinx-doc:4.x Nov 10, 2021
17 checks passed
@tk0miya
Copy link
Member

@tk0miya tk0miya commented Nov 10, 2021

Thank you for update. Merging. I'll fix the warning type from my side.

tk0miya added a commit that referenced this issue Nov 10, 2021
Fix the type of deprecation warning for get_signature_prefix (refs: #9833)
@jakobandersen
Copy link
Contributor Author

@jakobandersen jakobandersen commented Nov 10, 2021

Great, thanks!

@jakobandersen jakobandersen deleted the py_get_signature_prefix branch Nov 10, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants