8000 Add legend title font properties by smartlixx · Pull Request #19304 · matplotlib/matplotlib · GitHub 8000
[go: up one dir, main page]
Skip to content

Add legend title font properties #19304

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
Merged
QuLogic merged 3 commits into from
Apr 16, 2021
Merged

Add legend title font properties #19304

QuLogic merged 3 commits into from
Apr 16, 2021

Conversation

@smartlixx
Copy link
Contributor
@smartlixx smartlixx commented Jan 15, 2021 •
edited
Loading

PR Summary

Add a keyword title_fontproperties to matplotlib.pyplot.legend, so that we can prescribe font properties for legend's title. It can't be used together with the title_fontsize. This closes issue #19259.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • 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).

Now we can use the following codes to specify legend title font properties, and title_fontsize will override the size in title_prop.

import matplotlib.pyplot as plt
import matplotlib as mpl

myfont1 = mpl.font_manager.FontProperties(fname='/System/Library/Fonts/Supplemental/Arial Italic.ttf')
myfont2 = mpl.font_manager.FontProperties(fname='/System/Library/Fonts/Supplemental/Arial Bold.ttf',size=20)
myfont3 = mpl.font_manager.FontProperties(fname='/System/Library/Fonts/Supplemental/Arial Bold.ttf')

fig, [[ax1, ax2], [ax3, ax4]] = plt.subplots(2, 2, figsize=(8, 6))

ax1.scatter(1, 1, 40, label='point')
ax1.legend(title='location')

ax2.scatter(1, 1, 40, label='point')
ax2.legend(title='location', prop=myfont1, title_fontproperties=myfont2)

ax3.scatter(1, 1, 40, label='point')
ax3.legend(title='location', prop=myfont1, title_fontsize=20)

ax4.scatter(1, 1, 40, label='point')
ax4.legend(title='location', prop=myfont1, title_fontproperties={'weight':'bold', 'size':20})

plt.show()

The output is

test_legend_title_prop

github-actions[bot]
github-actions bot reviewed Jan 15, 2021
View reviewed changes
Copy link
@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

@jklymak
Copy link
Member
jklymak commented Jan 15, 2021

Thanks a lot, this seems good (without looking at it too hard). It would be nice if the new functionality were in our automated tests, it probably needs a "what's new" and maybe an example in the docs? Perhaps just extend an example that already sets the fontsize?

@jklymak jklymak added this to the v3.5.0 milestone Jan 15, 2021
jklymak
jklymak reviewed Jan 15, 2021
View reviewed changes
jklymak
jklymak reviewed Jan 15, 2021
View reviewed changes
@smartlixx
Copy link
Contributor Author
smartlixx commented Jan 16, 2021

I don't know how to address the failures of docs building. Could anyone help, please?

aitikgupta
aitikgupta reviewed Jan 16, 2021
View reviewed changes
Copy link
Contributor
@aitikgupta aitikgupta left a comment

Choose a reason for hiding this comment

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

Docs fail with following warnings:

/home/circleci/project/doc/users/next_whats_new/legend_title_prop_kwarg.rst:7: WARNING: Unexpected indentation.
/home/circleci/project/doc/users/next_whats_new/legend_title_prop_kwarg.rst:8: WARNING: Block quote ends without a blank line; unexpected unindent.

@smartlixx
Copy link
Contributor Author
smartlixx commented Jan 16, 2021

Docs fail with following warnings:

/home/circleci/project/doc/users/next_whats_new/legend_title_prop_kwarg.rst:7: WARNING: Unexpected indentation.
/home/circleci/project/doc/users/next_whats_new/legend_title_prop_kwarg.rst:8: WARNING: Block quote ends without a blank line; unexpected unindent.

Many thanks. I have accepted the suggested changes.

aitikgupta
aitikgupta reviewed Jan 16, 2021
View reviewed changes
jklymak
jklymak reviewed Jan 17, 2021
View reviewed changes
@smartlixx smartlixx changed the title Add lgend title properties Add legend title properties Jan 18, 2021
@smartlixx smartlixx changed the title Add legend title properties Add legend title font properties Jan 18, 2021
@smartlixx smartlixx requested a review from jklymak January 22, 2021 06:26
jklymak
jklymak approved these changes Jan 22, 2021
View reviewed changes
Copy link
Member
@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Thanks for sticking with it. Looks good to me, but we need a second review.

QuLogic
QuLogic reviewed Jan 22, 2021
View reviewed changes
Copy link
Member
@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

I suppose it might make sense to deprecate title_fontsize at this point? @timhoffm

< 8000 /form>
@smartlixx smartlixx requested a review from QuLogic January 22, 2021 13:04
tacaswell
tacaswell previously requested changes Jan 22, 2021
View reviewed changes
Copy link
Member
@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

I do not think we should add self.title_prop as new public API on the Legend.

@smartlixx smartlixx requested a review from tacaswell January 23, 2021 05:14
timhoffm
timhoffm reviewed Jan 25, 2021
View reviewed changes
@smartlixx smartlixx requested a review from timhoffm January 27, 2021 02:18
timhoffm
timhoffm reviewed Jan 28, 2021
View reviewed changes
@smartlixx smartlixx requested a review from timhoffm March 29, 2021 13:59
timhoffm
timhoffm reviewed Apr 11, 2021
View reviewed changes
Copy link
Member
@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

@smartlixx You've requested a review, however there are still unanswered comments from the previous review. Please address all questions before requesting a review.

@smartlixx
Copy link
Contributor Author
smartlixx commented Apr 13, 2021

@smartlixx You've requested a review, however there are still unanswered comments from the previous review. Please address all questions before requesting a review.

Thanks. I was waiting for your confirmation whether we should deprecate title_fontsize. Now that it is confirmed, I have made changes accordingly.

@smartlixx smartlixx force-pushed the add-lgend-title-properties branch from 9577434 to 3bcc6db Compare April 13, 2021 07:55
@smartlixx smartlixx requested a review from timhoffm April 13, 2021 12:43
timhoffm
timhoffm approved these changes Apr 13, 2021
View reviewed changes
Copy link
Member
@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Modulo one doc clarification.

@smartlixx smartlixx force-pushed the add-lgend-title-properties branch from 3bcc6db to 2aeed55 Compare April 13, 2021 23:00
@smartlixx
Copy link
Contributor Author
smartlixx commented Apr 14, 2021

@tacaswell can you please have a look? I still see the 'changes requested' label after I made those changes.

PS: Should I squash the commits to reduce the commit history?

QuLogic
QuLogic reviewed Apr 14, 2021
View reviewed changes
@QuLogic
Copy link
Member
QuLogic commented Apr 14, 2021

You will need to rebase to fix the doc build.

@smartlixx smartlixx force-pushed the add-lgend-title-properties branch 2 times, most recently from e55d23b to 5be2c74 Compare April 14, 2021 09:43
@smartlixx
Copy link
Contributor Author
smartlixx commented Apr 14, 2021

After rebase, the doc build still fails, at different places. And the Azure jobs failure is not related to this PR.

timhoffm
timhoffm reviewed Apr 14, 2021
View reviewed changes
Copy link
Member
@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Should fix the docs.

@smartlixx smartlixx force-pushed the add-lgend-title-properties branch 2 times, most recently from 775b01b to e7f9fb7 Compare April 14, 2021 14:03
@smartlixx
Add title_prop keyword to matplotlib.pyplot.legend
4b246ee
@smartlixx
Update lib/matplotlib/legend.py to simplify the logic as per suggeste…
655cffa 
…d by jklymak

Remove title_prop from public API and update test for robustness
Change title_prop to title_fontproperties
@smartlixx smartlixx force-pushed the add-lgend-title-properties branch from e7f9fb7 to 3e87c46 Compare April 14, 2021 15:00
QuLogic
QuLogic 8090 reviewed Apr 16, 2021
View reviewed changes
Copy link
Member
@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

Some small things could be shortened.

@smartlixx smartlixx force-pushed the add-lgend-title-properties branch from 3e87c46 to f5d44e0 Compare April 16, 2021 03:40
@QuLogic QuLogic dismissed tacaswell’s stale review April 16, 2021 04:43

self.title_prop is gone.

@QuLogic QuLogic merged commit 5799a5c into matplotlib:master Apr 16, 2021
@smartlixx smartlixx deleted the add-lgend-title-properties branch April 17, 2021 11:41
@timhoffm timhoffm mentioned this pull request Apr 29, 2021
Closed
@QuLogic QuLogic mentioned this pull request Jun 30, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuLogic QuLogic QuLogic left review comments

@jklymak jklymak jklymak approved these changes

@timhoffm timhoffm timhoffm approved these changes

@github-actions github-actions[bot] github-actions[bot] left review comments

@aitikgupta aitikgupta aitikgupta left review comments

@tacaswell tacaswell Awaiting requested review from tacaswell

Assignees

No one assigned

Labels

New feature topic: text/fonts

Projects

None yet

Milestone

v3.5.0

Development

Successfully merging this pull request may close these issues.

6 participants

@smartlixx @jklymak @QuLogic @tacaswell @timhoffm @aitikgupta
0