-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC: make legend docstring interpolated #10799
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
DOC: make legend docstring interpolated #10799
Conversation
Does "Other Parameters" not behave the same as Parameters? Or is this an indenting issue? |
OTOH, this looks the same as the current docs, so I'll argue is an improvement. https://matplotlib.org/api/_as_gen/matplotlib.axes.Axes.legend.html#matplotlib.axes.Axes.legend |
It should behave the same. Probably an indenting issue. |
OK, well, I'll try to hack until its fixed... |
@timhoffm when I had this stuff hardcoded in (before this change) it also failed to look the same as Parameters. So, I'm not sure this PR isn't OK as is, though I'd be happy to fix the issue if its easy to fix. |
9b359ce
to
da1763c
Compare
Are we talking about the same thing? https://7779-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/api/legend_api.html has issues with the handles and labels parameters, whereas https://matplotlib.org/devdocs/api/legend_api.html?highlight=legend#module-matplotlib.legend_handler Does not. |
I agree with that. But it had issues before this PR as well. Ie look at the current version of the docs (linked above). So the problem isn’t the docstring interpolation. OTOH I’m willing to fix the problem, I just have no idea what it is |
@timhoffm all the "Other Parameter" blocks look like this one: i.e. https://matplotlib.org/api/_as_gen/matplotlib.axes.Axes.text.html#matplotlib.axes.Axes.text So I think thats just how other-parameters ends up looking. |
@timhoffm Thats indeed what I meant. This PR doesn't make the formatting worse, and makes the docstring currating substantially easier. The "Other Parameters" bug is a different battle. |
The custom dictionary mapping instances or types to a legend | ||
handler. This `handler_map` updates the default handler map | ||
found at :func:`matplotlib.legend.Legend.get_legend_handler_map`. | ||
%(_legend_kw_doc)s |
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.
vs
https://matplotlib.org/api/legend_api.html#matplotlib.legend.Legend
Something is not right here, the handles and labels parameters are being extra indented, but I can not see why....
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.
https://matplotlib.org/devdocs/api/legend_api.html#matplotlib.legend.Legend devdocs also look ok....
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.
I see what you and @timhoffm mean now - you mean in the Parameter
section above this change. Silly me, only checking the part of the doctoring I edited...
I cannot see why either. Very strange as the Axes.legend
and Figure.legend
docstrings are fine, and I don't see how they are any different than this doctoring...
d26e1b4
to
6888d5d
Compare
@@ -348,7 +348,6 @@ def __init__(self, parent, handles, labels, | |||
handler_map=None, | |||
): | |||
""" | |||
|
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.
This extra space was causing the hiccup in the formatting. Sphinx is pretty fragile....
OK, fixed: https://7929-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/api/legend_api.html |
PR Summary
The three legend docstrings (
legend.py
,ax.legend
,fig.legend
) have the same "Other Parameters" section. These had drifted apart over the years (as had their acceptable kwargs). We unified them #9324, but there was some protest about using the docstring interp method, so I cut and pasted the docstrings, and added a test totest_legend
that made sure the strings were identical.Not surprisingly, this has caused problems. The test routinely fails when someone has edited one of the three docstrings. Good, its catching inconsistencies, but its a bit hard to tell folks to cut and paste the same thing three time.
I (somewhat strongly) think docstring interpolation is the better option here. Faced with the mysteries of
%(_legend_kw_doc)s
I think its save to assume the naive user will search the code base for this string and see thats where to put their new description of (yet another) kwarg for legend.Could be convinced that just referencing
legend.__init__
would be OK, but I do like the kwargs to appear in the local docstring.PR Checklist