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

Contour fixes/improvements #16171

Merged
merged 5 commits into from
Nov 10, 2020
Merged

Contour fixes/improvements #16171

merged 5 commits into from
Nov 10, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 9, 2020

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

# For closed polygons, add extra point to avoid division by
# zero in print_label and locate_label. Other than these
# functions, this is not necessary and should probably be
# eventually removed.
which shifts the labels a bit but has no bad effects (there is no division in print_label and division by zero warning is expressly suppressed in locate_label.

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

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

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 anntzer force-pushed the contour branch 4 times, most recently from f11f36a to 113c652 Compare November 6, 2020 18:07
@anntzer anntzer marked this pull request as ready for review November 6, 2020 18:08
Contour plots now default to using ScalarFormatter
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Pass ``fmt="%1.3f"`` to the countouring call to restore the old default label
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

It says coutouring now...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

second try...

lib/matplotlib/contour.py Outdated Show resolved Hide resolved
@QuLogic QuLogic added this to the v3.4.0 milestone Nov 10, 2020
@QuLogic QuLogic merged commit 1ed5c91 into matplotlib:master Nov 10, 2020
@anntzer anntzer deleted the contour branch November 10, 2020 09:34
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.

gap size for contour labels is poorly estimated
3 participants