Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 toda1763cCompareAre 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 theParameter sectionabove this change. Silly me, only checking the part of the doctoring I edited...
I cannot see why either. Very strange as theAxes.legend andFigure.legend docstrings are fine, and I don't see how they are any different than this doctoring...
d26e1b4 to6888d5dCompare| @@ -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 |
Uh oh!
There was an error while loading.Please reload this page.
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_legendthat 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)sI 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