-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Suptitle swapping order of updates between defaults and fontdict #27707
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
base: main
Are you sure you want to change the base?
Conversation
This should fix issue matplotlib#27630
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
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.
Can you add a test to our test suite to make sure this works and continues to work? Probably in lib/matplotlib/tests//test_figure.py
.
395d719
to
4a78e56
Compare
@jklymak, I added a test. Let me know if this needs anything else. |
Is it possible to test without an image test by querying the fontsize? We try to minimize image tests, particularly with text, but sometimes it is unavoidable. |
4a78e56
to
cd17206
Compare
@jklymak, made this change. Thank you for the guidance! |
The new test does not appear to be passing. |
Closes #27630
PR summary
Issue #27630 highlighted how passing a
size
value insidefontdict
was being ignored withinfigure.suptitle
. This was happening because the default kwargs sent to Text were being updated with the defaults last (i.e. highest priority). Switching the order of the dictionary update fixes this, giving highest priority to arguments specified infontdict
by the user.Test code by @boffi:
Broken behavior:

Fixed behavior:

PR checklist
Note: This is my first PR. Please let me know if I messed up anything here.