Contour fixes/improvements#16171
Merged
QuLogic merged 5 commits intomatplotlib:masterfrom Nov 10, 2020
Merged
Conversation
6 tasks
7 tasks
The previous heuristic was wrong for "wide" fonts, and was also overly complicated, as the Text class already has the relevant functionality.
(locate_label has no division, print_label explicitly suppresses the division by zero warning.)
anntzer
force-pushed
the
contour
branch
4 times, most recently
from
f11f36a to
113c652
Compare
timhoffm
approved these changes
Nov 6, 2020
QuLogic
reviewed
Nov 10, 2020
| Contour plots now default to using ScalarFormatter | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Pass ``fmt="%1.3f"`` to the countouring call to restore the old default label |
Member
There was a problem hiding this comment.
Suggested change
| Pass ``fmt="%1.3f"`` to the countouring call to restore the old default label | |
| Pass ``fmt="%1.3f"`` to the contouring call to restore the old default label |
QuLogic
approved these changes
Nov 10, 2020
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Closes #16153, and does some more.
Three fixes/improvements to contour labeling, grouped together to minimize baseline images changes.
Code-wise I think this is ready for review, but marking as draft PR just so that I can still have a look at whether we can combine/delete some of the baseline images.I also took advantage of the baseline regen to merge two tests (test_contour_manual_labels and test_contour_label_size_color).Commit 1: Improve estimation of label text width, as explained in #16153.
Commit 2: Remove
matplotlib/lib/matplotlib/contour.py
Lines 564 to 567 in 262c67f
Commit 3: Switch the default formatter to ScalarFormatter and use format_ticks to format all ticks at once, allowing the reuse of auto-precision choice by ScalarFormatter. Ultimately we could even use someting like #12473 (reusing colorbar logic to format values) but this shouldn't change any baseline images as none of them uses log scale (so they would still all use ScalarFormatter).
Commit 4: Update baseline images all at once. See the precision change and unicode minus sign in e.g. test_contour/contour_test_label_transforms.png and the tighter fit of labels in e.g. test_patheffects/collection.png.
I'm not sure why CI is failing even though I updated the baseline images locally, though.Figured out the test failure.PR Checklist