Skip to content

Improve formatting of imshow() cursor data independently of colorbar. #20949

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

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Aug 30, 2021

Currently (since #12459), when a colorbar is present, the cursor data under imshow() is
formatted using the colorbar's cursor formatter (the idea being that
that formatter should be able to "smartly" take normalization limits
into account); if no colorbar is present a fixed format string ("%0.3g")
is used (see #12473 for a stalled attempt to work around that).

In fact, there is a better scale that defines the number of significant
digits one should display in an imshow cursor data: it arises because
colormaps are discrete (usually with 256 colors, but in any case the
value is available as cmap.N). This quantization tells us, for a
given value, by how much one needs to move before the underlying color
changes (at all); that step size can be used to to determine a number
of significant digits to display. (Even if that's not necessarily
always the best number, it should at least be reasonable given the
user's choice of normalization
.)

Also, note that because ScalarFormatter has now changed to take pixel
size into account when determining its number of significant digits (#16776),
the previous approach of relying on the colorbar formatter has become
a less good approximation, as that means that the number of digits
displayed for an imshow() cursor could depend on the physical size of
the associated colorbar (if present).

Also factor out and reuse some logic to compute the number of
significant digits to use in format strings for given value/error pairs,
already used by the linear and polar tickers.

PR Summary

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).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

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.

This makes sense to me.

@anntzer anntzer force-pushed the smd branch 2 times, most recently from d280424 to b00423f Compare September 8, 2021 12:26
@@ -297,3 +298,20 @@ def warn_external(message, category=None):
break
frame = frame.f_back
warnings.warn(message, category, stacklevel)


def g_sig_digits(value, delta):
Copy link
Member

Choose a reason for hiding this comment

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

This function is not API related. I think it should be cbook._g_sig_digits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Currently, when a colorbar is present, the cursor data under imshow() is
formatted using the colorbar's cursor formatter (the idea being that
that formatter should be able to "smartly" take normalization limits
into account); if no colorbar is present a fixed format string ("%0.3g")
is used.

In fact, there is a better scale that defines the number of significant
digits one should display in an imshow cursor data: it arises because
colormaps are discrete (usually with 256 colors, but in any case the
value is available as `cmap.N`).  This quantization tells us, for a
given value, by how much one needs to move before the underlying color
changes (at all); that step size can be used to to determine a number
of significant digits to display.  (Even if that's not necessarily
always the best number, it should at least be reasonable *given the
user's choice of normalization*.)

Also, note that because ScalarFormatter has now changed to take pixel
size into account when determining *its* number of significant digits,
the previous approach of relying on the colorbar formatter has become
a less good approximation, as that means that the number of digits
displayed for an imshow() cursor could depend on the physical size of
the associated colorbar (if present).

Also factor out and reuse some logic to compute the number of
significant digits to use in format strings for given value/error pairs,
already used by the linear and polar tickers.
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.

Anybody can merge after CI pass.

@jklymak jklymak merged commit 0666c59 into matplotlib:master Sep 10, 2021
@jklymak jklymak added this to the v3.5.0 milestone Sep 10, 2021
@anntzer anntzer deleted the smd branch September 10, 2021 08:26
@jklymak
Copy link
Member

jklymak commented Sep 10, 2021

@meeseeksdev backport to 3.5.x

@lumberbot-app
Copy link

lumberbot-app bot commented Sep 10, 2021

Something went wrong ... Please have a look at my logs.

It seem that the branch you are trying to backport to does not exists.

@jklymak
Copy link
Member

jklymak commented Sep 10, 2021

@meeseeksdev backport to v3.5.x

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 10, 2021
timhoffm added a commit that referenced this pull request Sep 10, 2021
…949-on-v3.5.x

Backport PR #20949 on branch v3.5.x (Improve formatting of imshow() cursor data independently of colorbar.)
@anntzer anntzer mentioned this pull request Sep 21, 2021
7 tasks
tacaswell pushed a commit that referenced this pull request Oct 20, 2021
Improve formatting of imshow() cursor data independently of colorbar.
ericpre pushed a commit to ericpre/matplotlib that referenced this pull request Oct 20, 2021
Improve formatting of imshow() cursor data independently of colorbar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants