FIX: improve symlog ticker #21096

jklymak · 2021-09-16T12:18:14Z

PR Summary

SymLogLocator is supposed to not show any ticks in the range [-linthres, linthresh], except for zero. However, the floor call can leave a tick inside the lin threshold region, and this usually looks quite bad.

import numpy as np
import matplotlib.pyplot as plt

fig, ax = plt.subplots(figsize=(3, 3))
ax.plot(np.linspace(-1, 1, 1000))
ax.set_yscale('symlog', linthresh=0.03)
plt.show()

Before

After:

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).

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

closes matplotlib#20001

Fixes matplotlib#20516 PGF's `random steps` decoration seems to be the most similar, but does not exactly match the behaviour described in matplotlib's docs. Therefore I repurposed the `randomness` argument as a seed to give control on how the line looks afterwards.

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

Switch documented deprecations in mathtext by `__getattr__` deprecations

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

…_output API: rename draw_no_output to draw_without_rendering

Make warning for no-handles legend more explicit.

Remove unused HostAxes._get_legend_handles.

Redirect to new 3rd party packages page

legend_handler_map cleanups.

Clean up some Event class docs.

- parse() already normalizes prop=None to prop=FontProperties(). - We don't need to explicitly attach a canvas to a plain Figure() before calling savefig() anymore; FigureCanvasBase is always attached and handles the dispatching to the correct concrete canvas class.

According to the docs, this function was added in GTK 2.2, so probably was only needed when we supported GTK2. It also no longer exists in GTK4.

Since these examples don't need to support the very oldest GTK3, I took the opportunity to rewrite them using the recommended Application-styled model.

The `Gtk.Window.destroy` doesn't work, but `.close` correctly triggers our cleanup.

dstansby · 2021-09-16T13:43:53Z

I think the better fix is to change floor to ceil on line 2519 above? (which is what I did at the end of #14309, which never got merged)

jklymak · 2021-09-16T13:47:39Z

Maybe? We often return out of bounds ticks so I wasn't sure if that was desired here.

dstansby · 2021-09-16T15:46:41Z

What do you mean by out of bounds ticks? I think changing floor to ceil will have exactly the same effect as the current diff, and is makes more sense (in my opinion) when reading the code.

jklymak · 2021-09-17T10:55:27Z

Consider

import matplotlib.pyplot as plt 
import matplotlib.ticker as mticker

loc = mticker.LogLocator()
print(loc.tick_values(1e-3, 1e3))

This returns

[1.e-04 1.e-03 1.e-02 1.e-01 1.e+00 1.e+01 1.e+02 1.e+03 1.e+04]

and note that the first and last ticks are (well) outside the vmin and vmax.

dstansby · 2021-09-18T08:40:47Z

Ah I think that's because the hi lims should be using np.floor, not np.ceil? Essentially it looks like both calls should be switched in lines 2519-2520

jklymak · 2021-09-18T19:26:29Z

No, I mean the hi lim uses ceil because we typically want one tick above vmax.

dstansby · 2021-09-28T12:51:31Z

Sorry, I got a bit confused. to go back to my original suggestion, changing

matplotlib/lib/matplotlib/ticker.py

Line 2519 in a6e01b5

lo = np.floor(np.log(lo) / np.log(base))

from floor to ceil should fix the original issue. Using floor currently gives the first tick outside the log range; changing to ceil will give the first tick inside the log range.

jklymak · 2021-09-30T09:22:25Z

@dstansby We want one tick below vmin and one tick above vmax, and then we clip them when we draw the axis.

dstansby · 2021-09-30T09:28:31Z

In general yes, but that's not what this PR is about - the vmin/vmax of the Axis are distinct from the linear threshold of the ticker. As you say right at the top:

SymLogLocator is supposed to not show any ticks in the range [-linthres, linthresh], except for zero. However, the floor call can leave a tick inside the lin threshold region, and this usually looks quite bad.

Changing the

matplotlib/lib/matplotlib/ticker.py

Line 2519 in a6e01b5

lo = np.floor(np.log(lo) / np.log(base))

to ceil solves the issue, and is a much simpler and easy to follow piece of code than what is currently proposed.

jklymak · 2021-09-30T09:30:34Z

That same code is used above and below the threshold...

dstansby · 2021-09-30T09:36:30Z

I don't think we're doing a very good job of convincing each other 😄 - I won't block this, but leave for others to review.

jklymak · 2021-09-30T09:43:51Z

Well let me try again: get_log_range is used for both range_a and range_c So the hi part of range_a is thresh, but the hi part of range_c is vmax. So you would want to use floor and for range_a you'd want to use ceil for range_c. We could just get rid of get_log_range and write it all out, but that seemed more invasive than clipping the existing result at thresh.

dstansby · 2021-09-30T09:52:43Z

For the a and c ranges the calls are:

get_log_range(abs(a_upper_lim), abs(vmin) + 1)
get_log_range(c_lower_lim, vmax + 1)

The first argument for both the a and c ranges is linthresh (if linthresh is within the Axis limits), so applying ceil to the first argument will have the correct effect for both the a and c ranges, ie. returning the first tick outside the linear range.

jklymak · 2021-09-30T10:49:27Z

Oh duh, my apologies, you are absolutely correct. I've fixed it. Sorry for being so dense...

dstansby · 2021-09-30T11:09:03Z

Don't worry! Sorry for not explaining very well!

jklymak · 2021-09-30T11:32:15Z

Hmm, except that fails symlog2 test. Which I agree I've updated, but I think the update is better than before...

Master

Suggested by @dstansby

My suggestion

I guess not of these are quite right (i.e. the first axes should have more than just a zero tick!)

dstansby · 2021-09-30T12:25:32Z

Ahh I think this is because of the floating point issues using np.log instead of np.log10 when linthresh lies exactly on a decade. So I think changing log to log10 will fix it for base 10 (in #21177 ut seemed to work for other bases too) , but given this maybe it's better to go back to floor and clipping the ticks after all...

I guess not of these are quite right (i.e. the first axes should have more than just a zero tick!)

From memory, in this case the locator should just end up using the default linear locator?

tacaswell · 2021-10-20T20:33:41Z

This PR is affected by a re-writing of our history to remove a large number of accidentally committed files see discourse for details.

To recover this PR it will need be rebased onto the new default branch (main). There are several ways to accomplish this, but we recommend (assuming that you call the matplotlib/matplotlib remote "upstream"

git remote update
git checkout main
git merge --ff-only upstream/main
git checkout YOUR_BRANCH
git rebase --onto=main upstream/old_master
# git rebase -i main # if you prefer
git push --force-with-lease   # assuming you are tracking your branch

If you do not feel comfortable doing this or need any help please reach out to any of the Matplotlib developers. We can either help you with the process or do it for you.

Thank you for your contributions to Matplotlib and sorry for the inconvenience.

story645 and others added 30 commits August 26, 2021 23:06

Update lib/matplotlib/colors.py

a248e60

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

Update lib/matplotlib/colors.py

6529020

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

API: rename draw_no_output to draw_without_rendering

5821a24

closes matplotlib#20001

DOC: re-word the whats new entry

1a9f0a2

Fix docstring

f67e604

Add test

92e9fb6

Clarify stuff

c2ef020

Fix rendering of code block

9d73118

Automatically load PGF components

b94b282

Scale params to match existing appearance

18c06c3

Use descriptive names

a9377cd

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

Fix comment length

c3503a2

Merge pull request matplotlib#20944 from anntzer/mtd

3263061

Switch documented deprecations in mathtext by `__getattr__` deprecations

DOC: re-word whats new

124ba6e

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

Merge pull request matplotlib#20931 from tacaswell/api_rename_draw_no…

6d56592

…_output API: rename draw_no_output to draw_without_rendering

Merge pull request matplotlib#20956 from anntzer/lhu

ceb794c

Make warning for no-handles legend more explicit.

Merge pull request matplotlib#20955 from anntzer/glh

b5058a5

Remove unused HostAxes._get_legend_handles.

Merge pull request matplotlib#20952 from dstansby/3rd-party-redirect

bb12efe

Redirect to new 3rd party packages page

Merge pull request matplotlib#20957 from anntzer/lhm

6683517

legend_handler_map cleanups.

Merge pull request matplotlib#20745 from QuLogic/event-docs

a142369

Clean up some Event class docs.

BUG: Fix f_back is None handling

486ad06

Add a GTK4 backend.

0b724bf

Move common GTK Backend code into a separate file.

139a4db

Combine common GTK timer code.

e135227

Combine common GTK toolbar code.

1d6fd65

Clean up calls to Gtk.Window.set_icon_from_file.

68b4ee3

According to the docs, this function was added in GTK 2.2, so probably was only needed when we supported GTK2. It also no longer exists in GTK4.

Add GTK4 to docs.

7b63dee

Since these examples don't need to support the very oldest GTK3, I took the opportunity to rewrite them using the recommended Application-styled model.

Fix Ctrl+C out of IPython on GTK4.

d3804c2

The `Gtk.Window.destroy` doesn't work, but `.close` correctly triggers our cleanup.

jklymak marked this pull request as ready for review September 17, 2021 11:09

jklymak added this to the v3.5.1 milestone Sep 24, 2021

FIX: ceil instead of floor

ce4fef1

jklymak marked this pull request as draft September 30, 2021 11:32

tacaswell force-pushed the main branch from f6e0ee4 to a1eef38 Compare October 20, 2021 19:55

QuLogic modified the milestones: v3.5.1, v3.5.2 Dec 10, 2021

QuLogic added the status: needs rebase label Mar 29, 2022

jklymak modified the milestones: v3.5.2, v3.7.0 Mar 31, 2022

jklymak closed this Dec 11, 2022

jklymak deleted the fix-symlogscale branch December 11, 2022 21:41

Uh oh!

FIX: improve symlog ticker #21096

FIX: improve symlog ticker #21096

Uh oh!

Conversation

jklymak commented Sep 16, 2021

PR Summary

Before

After:

PR Checklist

Uh oh!

dstansby commented Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jklymak commented Sep 16, 2021

Uh oh!

dstansby commented Sep 16, 2021

Uh oh!

jklymak commented Sep 17, 2021

Uh oh!

dstansby commented Sep 18, 2021

Uh oh!

jklymak commented Sep 18, 2021

Uh oh!

dstansby commented Sep 28, 2021

Uh oh!

jklymak commented Sep 30, 2021

Uh oh!

dstansby commented Sep 30, 2021

Uh oh!

jklymak commented Sep 30, 2021

Uh oh!

dstansby commented Sep 30, 2021

Uh oh!

jklymak commented Sep 30, 2021

Uh oh!

dstansby commented Sep 30, 2021

Uh oh!

jklymak commented Sep 30, 2021

Uh oh!

dstansby commented Sep 30, 2021

Uh oh!

jklymak commented Sep 30, 2021

Master

Suggested by @dstansby

My suggestion

Uh oh!

dstansby commented Sep 30, 2021

Uh oh!

tacaswell commented Oct 20, 2021

Uh oh!

Uh oh!

dstansby commented Sep 16, 2021 •

edited

Loading