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 1 commit
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
Prev Previous commit
Next Next commit
Only use offset if it saves >=2 sig. digits. 
Tests cases courtesy of @WeatherGod.  Slightly improved numerical
accuracy.
  • Loading branch information
@anntzer
anntzer committed May 2, 2016
commit f37fd3da92c00d5a7e2fa17ed0bc061adf90670f
79 changes: 37 additions & 42 deletions lib/matplotlib/tests/test_ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,48 +164,43 @@ def test_ScalarFormatter_offset_value():
fig, ax = plt.subplots()
formatter = ax.get_xaxis().get_major_formatter()

def update_ticks(ax):
return next(ax.get_xaxis().iter_ticks())

ax.set_xlim(123, 189)
update_ticks(ax)
assert_equal(formatter.offset, 0)

ax.set_xlim(-189, -123)
update_ticks(ax)
assert_equal(formatter.offset, 0)

ax.set_xlim(12341, 12349)
update_ticks(ax)
assert_equal(formatter.offset, 12340)

ax.set_xlim(-12349, -12341)
update_ticks(ax)
assert_equal(formatter.offset, -12340)

ax.set_xlim(99999.5, 100010.5)
update_ticks(ax)
assert_equal(formatter.offset, 100000)

ax.set_xlim(-100010.5, -99999.5)
update_ticks(ax)
assert_equal(formatter.offset, -100000)

ax.set_xlim(99990.5, 100000.5)
update_ticks(ax)
assert_equal(formatter.offset, 100000)

ax.set_xlim(-100000.5, -99990.5)
update_ticks(ax)
assert_equal(formatter.offset, -100000)

ax.set_xlim(1233999, 1234001)
update_ticks(ax)
assert_equal(formatter.offset, 1234000)

ax.set_xlim(-1234001, -1233999)
update_ticks(ax)
assert_equal(formatter.offset, -1234000)
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),
# 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.



def _logfe_helper(formatter, base, locs, i, expected_result):
Expand Down
34 changes: 19 additions & 15 deletions lib/matplotlib/ticker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,34 +682,38 @@ def _compute_offset(self):
lmin, lmax = locs.min(), locs.max()
# min, max comparing absolute values (we want division to round towards
# zero so we work on absolute values).
abs_min, abs_max = sorted(map(abs, [lmin, lmax]))
# Only use offset if there are at least two ticks, every tick has the
# same sign, and if the span is small compared to the absolute values.
if (lmin == lmax or lmin <= 0 <= lmax or
(abs_max - abs_min) / abs_max >= 1e-2):
abs_min, abs_max = sorted([abs(float(lmin)), abs(float(lmax))])
# 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
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?
oom = 10 ** int(math.log10(abs_max) + 1)
# Note: Internally using oom instead of 10 ** oom avoids some numerical
# accuracy issues.
oom = math.ceil(math.log10(abs_max))
while True:
if abs_min // oom != abs_max // oom:
oom *= 10
if abs_min // 10 ** oom != abs_max // 10 ** oom:
oom += 1
break
oom /= 10
if (abs_max - abs_min) / oom <= 1e-2:
oom -= 1
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
# at most 1 apart?
Copy link
Member

Choose a reason for hiding this comment

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

s/at/are at/

oom = 10 ** int(math.log10(abs_max) + 1)
oom = math.ceil(math.log10(abs_max))
Copy link
Member

Choose a reason for hiding this comment

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

Instead of re-calculating this (which is maybe expensive?), can you just increase oom until it meets the opposite condition?

Copy link
Member

Choose a reason for hiding this comment

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

(Assuming you won't get an infinite loop somehow...)

while True:
if abs_max // oom - abs_min // oom > 1:
oom *= 10
if abs_max // 10 ** oom - abs_min // 10 ** oom > 1:
oom += 1
break
oom /= 10
self.offset = sign * (abs_max // oom) * oom
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