-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Promote mpltype Sphinx role to a public extension #28289
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
Conversation
Re: domain. As long as we can declare this non-public (use for inherited docstrings, but not in 3rd party code) we are still free to change the role names in the future, which means, we can wait until the (rather unlikely) case of a conflict with the global names happen and only then switch to domains. |
I am wary of saying things are private without a technical (or very obvious naming) way for the user to tell at the point they are using it. |
Well, it’s not an average user. Affected are only downstream projects that inherit from our classes and generate documentation. Maybe we could/should have guidelines for them anyway. I‘m hesitant to make the roles public unconditionally because they are primarily intended for internal use. Making them public would make it more difficult to make any changes. |
When projects derive from our types, but don't override all the docstrings, they may want to use these extensions so that their docs build.
I added an API change note now, but did not add this anywhere to our docs otherwise. |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
For third-party packages that derive types from Matplotlib, our use of custom roles may | ||
prevent Sphinx from building their docs. These custom Sphinx roles are now public solely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can be more explicit here:
- explain the problem: roles in inherited docstings
- Explicitly write down the exceptions for all our roles („you may see one on the following exceptions). Since we cannot influence the exception messages and they are not telling by themselves, people searching for the message string is their only way forward. We thus should have the stings found in our docs (and that hopefully later also via google).
- State that the addition of the extension should be used to solve the inherited docstring issue, but people should not use these roles explicitly in their docs. (Or rather, we don’t give any guarantees on these. - It might still be convenient to use them, but people should be aware that their docs may break in the future and they might have to adapt, which is not the end of the world for doc builds).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move the bulk of the explanation to the roles module docstring and give only a very brief summary here, linking to the module docs. That way, it’s easier to update the docs later.
@QuLogic I took the liberty to push a commit with the above suggested changes. Feel free to adapt as you see fit. |
…289-on-v3.9.x Backport PR #28289 on branch v3.9.x (Promote mpltype Sphinx role to a public extension)
PR summary
When projects derive from our types, but don't override all the docstrings, they may want to use these extensions so that their docs build. Fixes #28234
However, this does mean that when enabled, you get
:mpltype:
and:rc:
roles added globally. I have not fully investigated it, but we may want to instead move to a Sphinx Domain. This would require changing the usage to something like:mpl:type:
and:mpl:rc:
. However, I do need to investigate how that will interact with intersphinx (as if it doesn't work, then we don't really help downstreams.)PR checklist