Skip to content
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

Change legend guide to object oriented approach #20792

Merged
merged 1 commit into from Aug 6, 2021

Conversation

dmatos2012
Copy link
Contributor

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).

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if one of the plt examples should be left in as a resource but otherwise is awesome.

tutorials/intermediate/legend_guide.py Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
@dmatos2012
Copy link
Contributor Author

dmatos2012 commented Aug 4, 2021

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 🤦‍♂️

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
tutorials/intermediate/legend_guide.py Outdated Show resolved Hide resolved
@dmatos2012
Copy link
Contributor Author

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 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.

@jklymak jklymak merged commit e044cf5 into matplotlib:master Aug 6, 2021
@dmatos2012 dmatos2012 deleted the legend-tutorial-to-oo branch August 8, 2021 08:53
@QuLogic QuLogic added this to the v3.5.0 milestone Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants