Skip to content

/ matplotlib

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

Jump to bottom

Change legend guide to object oriented approach #20792

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Change legend guide to object oriented approach #20792

wants to merge 1 commit into from
+52 −42

Conversation

@dmatos2012
Copy link
Contributor

@dmatos2012 dmatos2012 commented Aug 4, 2021

PR Summary

Addresses #20754 to change legend guide from all pyplot interface to the ax objected oriented approach

PR Checklist

  • [N/A] Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [N/A] New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • [N/A] Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • [N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
@story645
story645 reviewed Aug 4, 2021
View changes
Copy link
Member

@story645 story645 left a comment

Not sure if one of the plt examples should be left in as a resource but otherwise is awesome.
tutorials/intermediate/legend_guide.py
line_up, = plt.plot([1, 2, 3], label='Line 2')
line_down, = plt.plot([3, 2, 1], label='Line 1')
plt.legend(handles=[line_up, line_down])
line_up, = ax.plot([1, 2, 3], label='Line 2')

This comment has been minimized.

Sign in to view
@story645

story645 Aug 4, 2021
Member

I think needs a 'fig, ax = plt.subplots()' to be runnable
tutorials/intermediate/legend_guide.py Outdated
plt.subplot(211)
plt.plot([1, 2, 3], label="test1")
plt.plot([3, 2, 1], label="test2")
ax1 = plt.subplot(211)

This comment has been minimized.

Sign in to view
@story645

story645 Aug 4, 2021
Member

fig, (ax1,ax2) = plt.subplots(nrows=2)
@dmatos2012 dmatos2012 force-pushed the dmatos2012:legend-tutorial-to-oo branch from 84a7c7a to 9d821e5 Aug 4, 2021
@dmatos2012
Copy link
Contributor Author

@dmatos2012 dmatos2012 commented Aug 4, 2021 •
edited

thanks @story645 for the review. I've updated the docs acording to the reviews. However, as per issue #20553, I think it would be nice to have some sort of description of the different loc descriptions for axes. It can be quite tricky to understand at first.

edit: flake8 linting 🤦‍♂️
@dmatos2012 dmatos2012 force-pushed the dmatos2012:legend-tutorial-to-oo branch from 9d821e5 to 078a87e Aug 4, 2021
@story645
story645 reviewed Aug 5, 2021
View changes
Copy link
Member

@story645 story645 left a comment

totally agree w/ you about adding in something on location, possibly as an expansion of https://62747-1385122-gh.circle-artifacts.com/0/doc/build/html/tutorials/intermediate/legend_guide.html#legend-location & maybe using the cheatsheet as inspiration?
https://github.com/matplotlib/cheatsheets/blob/master/cheatsheets-2.png
tutorials/intermediate/legend_guide.py
line_up, = plt.plot([1, 2, 3], label='Line 2')
line_down, = plt.plot([3, 2, 1], label='Line 1')
plt.legend([line_up, line_down], ['Line Up', 'Line Down'])
line_up, = ax.plot([1, 2, 3], label='Line 2')

This comment has been minimized.

Sign in to view
@story645

story645 Aug 5, 2021
Member

Suggested change
line_up, = ax.plot([1, 2, 3], label='Line 2')
fig, ax = plt.subplots()
line_up, = ax.plot([1, 2, 3], label='Line 2')

yeah is repetitive but I think they're all supposed to be self contained
@QuLogic
QuLogic reviewed Aug 5, 2021
View changes
tutorials/intermediate/legend_guide.py Outdated
# plt.legend(bbox_to_anchor=(1, 1),
# bbox_transform=plt.gcf().transFigure)
# ax.legend(bbox_to_anchor=(1, 1),
# bbox_transform=fig.transFigure)

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Re-align.
tutorials/intermediate/legend_guide.py Outdated
plt.legend(bbox_to_anchor=(1.05, 1), loc='upper left', borderaxespad=0.)
ax2.plot([1, 2, 3], label="test1")
ax2.plot([3, 2, 1], label="test2")
# Place a legend on top center of this subplot.

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

I think the point is to be outside on this one. We can replicate the layout with funny subplot mixing, using subplot_mosaic:

fig, ax_dict = plt.subplot_mosaic([
    ['top', 'top'],
    ['bottom', '-'],
])

This comment has been minimized.

Sign in to view
@dmatos2012

dmatos2012 Aug 5, 2021
Author Contributor

This is actually pretty cool, wasnt aware of that, bc I was struggling with using fig, ax = plt.subplots() to get that layout, hence changing the label to the inside. Thx for the tip @QuLogic
tutorials/intermediate/legend_guide.py Outdated

# Create a legend for the first line.
first_legend = plt.legend(handles=[line1], loc='upper right')
first_legend = ax.legend(handles=[line1], loc='upper right')

# Add the legend manually to the current Axes.

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Suggested change
# Add the legend manually to the current Axes.
# Add the legend manually to the Axes.
tutorials/intermediate/legend_guide.py Outdated
fig, ax = plt.subplots()
z = randn(10)

red_dot, = plt.plot(z, "ro", markersize=15)
red_dot, = ax.plot(z, "ro", markersize=15)
Comment on lines 213 to 217

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Suggested change
fig, ax = plt.subplots()
z = randn(10)
red_dot, = plt.plot(z, "ro", markersize=15)
red_dot, = ax.plot(z, "ro", markersize=15)
z = randn(10)
fig, ax = plt.subplots()
red_dot, = ax.plot(z, "ro", markersize=15)
tutorials/intermediate/legend_guide.py Outdated
l = ax.legend([(p1, p2)], ['Two keys'], numpoints=1,
handler_map={tuple: HandlerTuple(ndivide=None)})
Comment on lines 231 to 232

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Suggested change
l = ax.legend([(p1, p2)], ['Two keys'], numpoints=1,
handler_map={tuple: HandlerTuple(ndivide=None)})
l = ax.legend([(p1, p2)], ['Two keys'], numpoints=1,
handler_map={tuple: HandlerTuple(ndivide=None)})
tutorials/intermediate/legend_guide.py Outdated
@@ -238,6 +243,8 @@

import matplotlib.patches as mpatches

fig, ax = plt.subplots()

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Move down to where the legend is done.
tutorials/intermediate/legend_guide.py Outdated
@@ -272,6 +279,8 @@ def legend_artist(self, legend, orig_handle, fontsize, handlebox):

from matplotlib.legend_handler import HandlerPatch

fig, ax = plt.subplots()

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Move down.
tutorials/intermediate/legend_guide.py Outdated
ax.legend([AnyObject()], ['My first handler'],
handler_map={AnyObject: AnyObjectHandler()})
Comment on lines 264 to 265

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Suggested change
ax.legend([AnyObject()], ['My first handler'],
handler_map={AnyObject: AnyObjectHandler()})
ax.legend([AnyObject()], ['My first handler'],
handler_map={AnyObject: AnyObjectHandler()})
tutorials/intermediate/legend_guide.py Outdated
ax.legend([c], ["An ellipse, not a rectangle"],
handler_map={mpatches.Circle: HandlerEllipse()})
Comment on lines 300 to 301

This comment has been minimized.

Sign in to view
@QuLogic

QuLogic Aug 5, 2021
Member

Suggested change
ax.legend([c], ["An ellipse, not a rectangle"],
handler_map={mpatches.Circle: HandlerEllipse()})
ax.legend([c], ["An ellipse, not a rectangle"],
handler_map={mpatches.Circle: HandlerEllipse()})
@dmatos2012
Change legend guide to object oriented approach
5cb4dcf
@dmatos2012 dmatos2012 force-pushed the dmatos2012:legend-tutorial-to-oo branch from 078a87e to 5cb4dcf Aug 5, 2021
@dmatos2012
Copy link
Contributor Author

@dmatos2012 dmatos2012 commented Aug 5, 2021

totally agree w/ you about adding in something on location, possibly as an expansion of https://62747-1385122-gh.circle-artifacts.com/0/doc/build/html/tutorials/intermediate/legend_guide.html#legend-location & maybe using the cheatsheet as inspiration?
https://github.com/matplotlib/cheatsheets/blob/master/cheatsheets-2.png

I was thinking perhaps something like creating a 3x3 (or 2 x2 w.e) subplot_mosaic and just placing the legend in the different loc positions, and then link to the cheatsheet.png for their future reference. But given that this is mentioned on a different issue, I was thinking on creating another PR just for that, unless you clearly want it on this one instead. lmk
@story645
Copy link
Member

@story645 story645 commented Aug 5, 2021

I think it's a good idea to leave it to another issue since that's gonna be a bigger change to the guide content so folks might have more opinions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic

@story645 story645

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@dmatos2012 @story645 @QuLogic