Better choice of offset-text. by anntzer · Pull Request #5785 · matplotlib/matplotlib · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 48 additions & 1 deletion lib/matplotlib/tests/test_ticker.py
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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