Skip to content

Documentation of AnchoredText's prop keyword argument is misleading #12467

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.

Sign up for GitHub

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

Jump to bottom
Closed
galenlynch opened this issue Oct 9, 2018 · 2 comments · Fixed by #12469
Closed

Documentation of AnchoredText's prop keyword argument is misleading #12467

galenlynch opened this issue Oct 9, 2018 · 2 comments · Fixed by #12469
Labels
Documentation
Milestone
v3.0.0-doc

Comments

@galenlynch
Copy link
Contributor

galenlynch commented Oct 9, 2018 •
edited
Loading

This bug is nearly identical to #12121, but affects AnchoredText instead of TextArea.

The documentation for AnchoredText strongly suggests that the keyword argument prop should be a FontProperties object. However, inspecting the source code suggests that prop should instead be a dictionary of keyword arguments that will be passed to the TextArea object inside of AnchoredText.

I propose rewording the documentation to make it clear that prop should be a dictionary of keyword arguments to construct the TextArea instance.

Perhaps it would be worthwhile to look over the documentation of other classes in offsetbox for similar issues, since this is the second time I've come across the same issue.
@galenlynch galenlynch changed the title Documentation of AnchoredText's props keyword argument is misleading Documentation of AnchoredText's prop keyword argument is misleading Oct 9, 2018
@ImportanceOfBeingErnest
Copy link
Member

Sure, judging from #12434 you already know how that would be fixed. So if you have the time, just go ahead (no need to open a new issue for each item you find).

@galenlynch
Copy link
Contributor Author

Maybe I should make another issue for this, but it would be nice if TextArea and AnchoredText had a consistent API, maybe by renaming the prop argument of AnchoredText to be fontprops as is used by TextArea, which has the added benefit of being more descriptive.

It seems like it would be fairly easy to also support the old keyword, to prevent breaking people's code.

What's the attitude around here to such API changes? All things considered, it's a very minor complaint.

galenlynch added a commit to galenlynch/matplotlib that referenced this issue Oct 9, 2018
@galenlynch
Clarify documentation of offsetbox.AnchoredText's prop kw argument
45f7d5d 
`prop` should be a dictionary of keyword arguments that will be passed to the
`Text` object contained in `AnchoredText` (via `TextArea`). The current
documentation suggests that it should instead be a `FontProperties` object.

This commit changes the documentation to reflect the current behavior.

closes matplotlib#12467
@galenlynch galenlynch mentioned this issue Oct 9, 2018
Merged
6 tasks
@QuLogic QuLogic added this to the v3.0.0-doc milestone Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.0.0-doc
Development

Successfully merging a pull request may close this issue.

Clarify documentation of offsetbox.AnchoredText's prop kw argument galenlynch/matplotlib
4 participants
@QuLogic @dstansby @galenlynch @ImportanceOfBeingErnest