Skip to content

Make LogLocator only return one tick out of range #24436

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

Make LogLocator only return one tick out of range #24436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d495301
Move invalid vmin to separate test
dstansby Nov 12, 2022
5e733de
Add some new LogLocator tests
dstansby Nov 12, 2022
4ba7411
Add note about out of bounds ticks
dstansby Dec 1, 2022
737aa02
Simplify log locator logic
dstansby Dec 1, 2022
36adb34
Fix tests
dstansby Dec 1, 2022
792236a
Add changelog entry
dstansby Dec 1, 2022
a8ba49d
Fix non-positive warning
dstansby Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/next_api_changes/behavior/24436-DS.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Improved tick location logic in ``LogLocator``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

`~.LogLocator.tick_values` previously returned two ticks that were outside
the range ``{vmin, vmax}``. This has been updated to only return all ticks
for the decades containing ``vmin`` and ``vmax``. If ``vmin`` or ``vmax``
are exactly on a decade this means no ticks above/below are returned.
Otherwise a single major tick is returned above/below, and for minor ticks
all the ticks in the decade containing ``vmin`` or ``vmax`` are returned.
Copy link
Member

Choose a reason for hiding this comment

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

Can this clarify the previous behaviour? Did we un-conditionally return an extra tick?

Copy link
Member

Choose a reason for hiding this comment

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

Before it was 1 or 2, now it is 0 or 1.

I think the question is MUST locators return extra or MAY locators return extra.

4 changes: 2 additions & 2 deletions lib/matplotlib/tests/test_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6419,8 +6419,8 @@ def test_auto_numticks_log():
fig, ax = plt.subplots()
mpl.rcParams['axes.autolimit_mode'] = 'round_numbers'
ax.loglog([1e-20, 1e5], [1e-16, 10])
assert (np.log10(ax.get_xticks()) == np.arange(-26, 18, 4)).all()
assert (np.log10(ax.get_yticks()) == np.arange(-20, 10, 3)).all()
assert_array_equal(np.log10(ax.get_xticks()), np.arange(-22, 14, 4))
assert_array_equal(np.log10(ax.get_yticks()), np.arange(-17, 7, 3))


def test_broken_barh_empty():
Expand Down
12 changes: 7 additions & 5 deletions lib/matplotlib/tests/test_colorbar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,10 @@ def test_colorbar_autotickslog():
orientation='vertical', shrink=0.4)
# note only -12 to +12 are visible
np.testing.assert_almost_equal(cbar.ax.yaxis.get_ticklocs(),
10**np.arange(-16., 16.2, 4.))
# note only -24 to +24 are visible
10**np.arange(-12., 12.2, 4.))
# note only -12 to +12 are visible
Copy link
Member

Choose a reason for hiding this comment

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

Was this comment wrong before?

np.testing.assert_almost_equal(cbar2.ax.yaxis.get_ticklocs(),
10**np.arange(-24., 25., 12.))
10**np.arange(-12., 13., 12.))


def test_colorbar_get_ticks():
Expand Down Expand Up @@ -584,6 +584,7 @@ def test_colorbar_log_minortick_labels():
def test_colorbar_renorm():
x, y = np.ogrid[-4:4:31j, -4:4:31j]
z = 120000*np.exp(-x**2 - y**2)
# min/max of z vals is 1.5196998658913011e-09, 120000.0

fig, ax = plt.subplots()
im = ax.imshow(z)
Expand All @@ -597,7 +598,7 @@ def test_colorbar_renorm():
norm = LogNorm(z.min(), z.max())
im.set_norm(norm)
np.testing.assert_allclose(cbar.ax.yaxis.get_majorticklocs(),
np.logspace(-10, 7, 18))
np.logspace(-9, 6, 16))
# note that set_norm removes the FixedLocator...
assert np.isclose(cbar.vmin, z.min())
cbar.set_ticks([1, 2, 3])
Expand All @@ -616,6 +617,7 @@ def test_colorbar_format(fmt):
# make sure that format is passed properly
x, y = np.ogrid[-4:4:31j, -4:4:31j]
z = 120000*np.exp(-x**2 - y**2)
# min/max of z vals is 1.5196998658913011e-09, 120000.0

fig, ax = plt.subplots()
im = ax.imshow(z)
Expand All @@ -633,7 +635,7 @@ def test_colorbar_format(fmt):
im.set_norm(LogNorm(vmin=0.1, vmax=10))
fig.canvas.draw()
assert (cbar.ax.yaxis.get_ticklabels()[0].get_text() ==
'$\\mathdefault{10^{-2}}$')
'$\\mathdefault{10^{-1}}$')


def test_colorbar_scale_reset():
Expand Down
36 changes: 28 additions & 8 deletions lib/matplotlib/tests/test_ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,28 +194,48 @@ def test_additional(self, lim, ref):


class TestLogLocator:
@pytest.mark.parametrize(
'vmin, vmax, expected_ticks',
(
[1, 10, [1, 10]],
[0.9, 10.1, [0.1, 1, 10, 100]],
[0.9, 9, [0.1, 1, 10]],
[1.1, 11, [1, 10, 100]],
[0.5, 983, [0.1, 1, 10, 100, 1000]]
)
)
def test_tick_locs(self, vmin, vmax, expected_ticks):
ll = mticker.LogLocator(base=10.0, subs=(1.0,))
np.testing.assert_equal(ll.tick_values(vmin, vmax), expected_ticks)

def test_basic(self):
loc = mticker.LogLocator(numticks=5)
with pytest.raises(ValueError):
loc.tick_values(0, 1000)

test_value = np.array([1.00000000e-05, 1.00000000e-03, 1.00000000e-01,
test_value = np.array([1.00000000e-03, 1.00000000e-01,
1.00000000e+01, 1.00000000e+03, 1.00000000e+05,
1.00000000e+07, 1.000000000e+09])
assert_almost_equal(loc.tick_values(0.001, 1.1e5), test_value)
1.00000000e+07])
assert_almost_equal(loc.tick_values(1e-3, 1.1e5), test_value)

loc = mticker.LogLocator(base=2)
test_value = np.array([0.5, 1., 2., 4., 8., 16., 32., 64., 128., 256.])
test_value = np.array([1., 2., 4., 8., 16., 32., 64., 128.])
assert_almost_equal(loc.tick_values(1, 100), test_value)

def test_invalid_lim_error(self):
loc = mticker.LogLocator()
with pytest.raises(
ValueError,
match='Data has non-positive values, '
'and therefore can not be log-scaled.'
):
loc.tick_values(0, 1000)

def test_polar_axes(self):
"""
Polar axes have a different ticking logic.
"""
fig, ax = plt.subplots(subplot_kw={'projection': 'polar'})
ax.set_yscale('log')
ax.set_ylim(1, 100)
assert_array_equal(ax.get_yticks(), [10, 100, 1000])
assert_array_equal(ax.get_yticks(), [10, 100])

def test_switch_to_autolocator(self):
loc = mticker.LogLocator(subs="all")
Expand Down
19 changes: 15 additions & 4 deletions lib/matplotlib/ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2344,6 +2344,15 @@ def __call__(self):
return self.tick_values(vmin, vmax)

def tick_values(self, vmin, vmax):
"""
Return the values of the located ticks given **vmin** and **vmax**.

Notes
-----
The tick values returned include all ticks for the decades containing
``vmin`` and ``vmax``, which can result in some tick values below
``vmin`` and above ``vmax``.
"""
if self.numticks == 'auto':
if self.axis is not None:
numticks = np.clip(self.axis.get_tick_space(), 2, 9)
Expand All @@ -2359,7 +2368,7 @@ def tick_values(self, vmin, vmax):

if vmin <= 0.0 or not np.isfinite(vmin):
raise ValueError(
"Data has no positive values, and therefore can not be "
"Data has non-positive values, and therefore can not be "
Copy link
Member

Choose a reason for hiding this comment

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

This overall logic does not seem 10% correct because we are only checking vmin here and just below we ensure that vmin and vmax are sorted.

We also only get this error if the locator is not attached to as axis as in that case we silently clip vmin.

"log-scaled.")

_log.debug('vmin %s vmax %s', vmin, vmax)
Expand Down Expand Up @@ -2399,8 +2408,10 @@ def tick_values(self, vmin, vmax):
# whether we're a major or a minor locator.
have_subs = len(subs) > 1 or (len(subs) == 1 and subs[0] != 1.0)

decades = np.arange(math.floor(log_vmin) - stride,
math.ceil(log_vmax) + 2 * stride, stride)
# Return one tick below (or equal to) vmin,
# and one tick above (or equal to) vmax
decades = np.arange(math.floor(log_vmin),
math.ceil(log_vmax) + stride, stride)

if hasattr(self, '_transform'):
ticklocs = self._transform.inverted().transform(decades)
Expand Down Expand Up @@ -2450,7 +2461,7 @@ def nonsingular(self, vmin, vmax):
vmin, vmax = 1, 10 # Initial range, no data plotted yet.
elif vmax <= 0:
_api.warn_external(
"Data has no positive values, and therefore cannot be "
"Data has non-positive values, and therefore cannot be "
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
"Data has non-positive values, and therefore cannot be "
"Data has no positive values, and therefore cannot be "

I think this one should stay as "no positive" as in the other blocks we clip vmin to a small positive value.

"log-scaled.")
vmin, vmax = 1, 10
else:
Expand Down