DOC: Fix doc builds with Sphinx 9#31020
Conversation
|
precommit check-docstring-first stumbles over this: pre-commit/pre-commit-hooks#159. It creates a false positive warning on module attribute docstrings. The issue is closed but unresolved. Options are:
It would likely also possible to improve check-docstring-first (essentially it could stop search when encountering the first assignment), but likely not worth the effort. |
This is generally frowned upon (though we don't actually change it, so it's safe), but more importantly, Sphinx tries to put all the values into the docstring, which just looks bad.
The new version appears to be causing some issues for our missing reference checks, so reverting back to the old implementation should buy us some time to fix things.
|
It seems unlikely that upstream will fix that check, since both the issue and the attempted PR were closed. I've just changed those to doc-comments, which seems to still work. Actually, when pre-commit fixes were applied in #22809, we removed the docstrings on these variables. I only added them back to give the defaults somewhere to link to, but we could just keep them undocumented. |
|
|
||
|
|
||
| def add_tools_to_container(container, tools=default_toolbar_tools): | ||
| def add_tools_to_container(container, tools=None): |
There was a problem hiding this comment.
This subtly changes semantics. Now it is possible to do
from matplotlib import backend_tools
backend_tools = {...}
and affect the used default. Before it was still possible to modify the default by in-place modification, but I haven't found any such usage on github.
Overall, I believe this is not intended to be configurable. Should we make it private? Or alternatively switch to DEFAULT_TOOLS if you want to reference the values in the documentation.
There was a problem hiding this comment.
Hmm, fair point, it is a bit more flexible, but someone could already do that if they wanted to as the default value is a reference:
>>> foo = ['a', 'b', 'c']
>>> def bar(baz=foo):
... print(baz)
...
>>> bar()
['a', 'b', 'c']
>>> foo.append('d') # Modify the global in-place.
>>> bar()
['a', 'b', 'c', 'd'] # Reflects the modification.There was a problem hiding this comment.
That's what I'be been referring to as "in-place" modification. But doesn't seem to be used in the wild.
There was a problem hiding this comment.
Ah sorry, I missed that. Well, I don't have any great need to document these. The main goal is removing them from the signature so Sphinx doesn't try to link the classes erroneously, and I've only linked them because previously you could see what the default was (if in a bit difficult to read manner.) If we don't necessarily want to document what the default is, then we can remove the docs for these, and possibly deprecate the variables.
There was a problem hiding this comment.
I've merged as is to unbreak the doc builds now.
The differences in handling tool defaults are relatively low-impact. Any variant is good enough for now. I'll think about the proper solution and made a tracking issue for that: #31027
2 participants
PR summary
This switches Sphinx 9 to using its old class-based autodoc implementation, and tweaks some code to avoid extra cross-references. This would not be future proof to the removal of the old implementation, but should buy us some time at least while working that out.
cf #31019 though I'm not marking it fixed as we should figure out how to get it working with the new implementation.
PR checklist