Skip to content

Better choice of offset-text. #5785

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

Merged
merged 5 commits into from
May 11, 2016
Merged

Better choice of offset-text. #5785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9a4ecfb
Better choice of offset-text.
anntzer Jan 2, 2016
f37fd3d
Only use offset if it saves >=2 sig. digits.
anntzer Jan 6, 2016
0c39a78
Slightly more efficient impl.; more tests.
anntzer Feb 18, 2016
9d0acb2
Fix test values.
anntzer May 2, 2016
782b8a1
Add What's new entry.
anntzer May 2, 2016
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
6 changes: 6 additions & 0 deletions doc/users/whats_new/offset-text-choice.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Improved offset text choice
---------------------------
The default offset-text choice was changed to only use significant digits that
are common to all ticks (e.g. 1231..1239 -> 1230, instead of 1231), except when
they straddle a relatively large multiple of a power of ten, in which case that
multiple is chosen (e.g. 1999..2001->2000).
49 changes: 48 additions & 1 deletion lib/matplotlib/tests/test_ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from matplotlib.externals import six
import nose.tools
from nose.tools import assert_raises
from nose.tools import assert_equal, assert_raises
from numpy.testing import assert_almost_equal
import numpy as np
import matplotlib
Expand Down Expand Up @@ -159,6 +159,53 @@ def test_SymmetricalLogLocator_set_params():
nose.tools.assert_equal(sym.numticks, 8)


@cleanup
def test_ScalarFormatter_offset_value():
Copy link
Member

Choose a reason for hiding this comment

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

This needs an @cleanup decorator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it is, the @cleanup decorator doesn't support generative tests. I'll write an patch for that first.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome. That is something that has driven me crazy a couple of times. I have dealt with it by putting the decorator on the called function and creating all of the figures/axes in that function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #5809.

fig, ax = plt.subplots()
formatter = ax.get_xaxis().get_major_formatter()

def check_offset_for(left, right, offset):
ax.set_xlim(left, right)
# Update ticks.
next(ax.get_xaxis().iter_ticks())
assert_equal(formatter.offset, offset)

test_data = [(123, 189, 0),
(-189, -123, 0),
(12341, 12349, 12340),
(-12349, -12341, -12340),
(99999.5, 100010.5, 100000),
(-100010.5, -99999.5, -100000),
(99990.5, 100000.5, 100000),
(-100000.5, -99990.5, -100000),
(1233999, 1234001, 1234000),
(-1234001, -1233999, -1234000),
(1, 1, 1),
(123, 123, 120),
# Test cases courtesy of @WeatherGod
(.4538, .4578, .45),
(3789.12, 3783.1, 3780),
(45124.3, 45831.75, 45000),
(0.000721, 0.0007243, 0.00072),
(12592.82, 12591.43, 12590),
(9., 12., 0),
(900., 1200., 0),
(1900., 1200., 0),
(0.99, 1.01, 1),
(9.99, 10.01, 10),
(99.99, 100.01, 100),
(5.99, 6.01, 6),
(15.99, 16.01, 16),
(-0.452, 0.492, 0),
(-0.492, 0.492, 0),
(12331.4, 12350.5, 12300),
(-12335.3, 12335.3, 0)]

for left, right, offset in test_data:
yield check_offset_for, left, right, offset
Copy link
Member

Choose a reason for hiding this comment

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

I think all but two test cases are left < right; would it also make sense to yield in the reverse order? Also, I don't think there are any tests for left == right (though I don't see why that won't work correctly.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really think I need to support left == right because I don't see how this can ever happen. Other issues handled in new (rebased) commit.

Copy link
Member

Choose a reason for hiding this comment

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

Well, your code does check for the left == right case, so it's good so codify what the expected behaviour is in that case. I think the default locators would avoid this situation, but I'm not sure about user-created locators.

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. I'll wait for #6022 to be merged in so that the tests are actuall run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

yield check_offset_for, right, left, offset


def _logfe_helper(formatter, base, locs, i, expected_result):
vals = base**locs
labels = [formatter(x, pos) for (x, pos) in zip(vals, i)]
Expand Down
54 changes: 36 additions & 18 deletions lib/matplotlib/ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@
from matplotlib.externals import six

import decimal
import itertools
import locale
import math
import numpy as np
Expand Down Expand Up @@ -663,33 +664,50 @@ def set_locs(self, locs):
vmin, vmax = self.axis.get_view_interval()
d = abs(vmax - vmin)
if self._useOffset:
self._set_offset(d)
self._compute_offset()
self._set_orderOfMagnitude(d)
self._set_format(vmin, vmax)

def _set_offset(self, range):
# offset of 20,001 is 20,000, for example
def _compute_offset(self):
Copy link

Choose a reason for hiding this comment

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

I fully agree that renaming this method was really needed; the new name actually reflects the purpose of the function.

Copy link
Member

Choose a reason for hiding this comment

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

Contra my reluctance to change the public API at all, changing anything with a leading _ is fair game.

locs = self.locs

if locs is None or not len(locs) or range == 0:
if locs is None or not len(locs):
self.offset = 0
return
# Restrict to visible ticks.
vmin, vmax = sorted(self.axis.get_view_interval())
locs = np.asarray(locs)
locs = locs[(vmin <= locs) & (locs <= vmax)]
ave_loc = np.mean(locs)
if len(locs) and ave_loc: # dont want to take log10(0)
ave_oom = math.floor(math.log10(np.mean(np.absolute(locs))))
range_oom = math.floor(math.log10(range))

if np.absolute(ave_oom - range_oom) >= 3: # four sig-figs
p10 = 10 ** range_oom
if ave_loc < 0:
self.offset = (math.ceil(np.max(locs) / p10) * p10)
else:
self.offset = (math.floor(np.min(locs) / p10) * p10)
else:
self.offset = 0
if not len(locs):
self.offset = 0
return
lmin, lmax = locs.min(), locs.max()
# Only use offset if there are at least two ticks and every tick has
# the same sign.
if lmin == lmax or lmin <= 0 <= lmax:
Copy link
Member

Choose a reason for hiding this comment

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

Can move this section above the absolute values to short-circuit a little earlier.

self.offset = 0
return
# min, max comparing absolute values (we want division to round towards
# zero so we work on absolute values).
abs_min, abs_max = sorted([abs(float(lmin)), abs(float(lmax))])
sign = math.copysign(1, lmin)
# What is the smallest power of ten such that abs_min and abs_max are
# equal up to that precision?
# Note: Internally using oom instead of 10 ** oom avoids some numerical
# accuracy issues.
oom_max = math.ceil(math.log10(abs_max))
oom = 1 + next(oom for oom in itertools.count(oom_max, -1)
if abs_min // 10 ** oom != abs_max // 10 ** oom)
if (abs_max - abs_min) / 10 ** oom <= 1e-2:
# Handle the case of straddling a multiple of a large power of ten
# (relative to the span).
# What is the smallest power of ten such that abs_min and abs_max
# are no more than 1 apart at that precision?
oom = 1 + next(oom for oom in itertools.count(oom_max, -1)
if abs_max // 10 ** oom - abs_min // 10 ** oom > 1)
# Only use offset if it saves at least two significant digits.
self.offset = (sign * (abs_max // 10 ** oom) * 10 ** oom
if abs_max // 10 ** oom >= 10
else 0)

def _set_orderOfMagnitude(self, range):
# if scientific notation is to be used, find the appropriate exponent
Expand Down