From da5796cd3b0a6632b2489d3fa3b80f0490c42b09 Mon Sep 17 00:00:00 2001 From: haval0 <56519858+haval0@users.noreply.github.com> Date: Wed, 1 Mar 2023 16:44:58 +0100 Subject: [PATCH 1/5] feat: add 2 tests based on good/bad plot from upstream issue The test cases are based on the demonstration of the issue in upstream issue #24092 The test case for bad plot (test_tick_values_not_empty) should fail until we implement our fix. The test case for good plot (test_tick_values_correct) should already suceed and continue to succeed after we implement our fix. --- lib/matplotlib/tests/test_ticker.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/matplotlib/tests/test_ticker.py b/lib/matplotlib/tests/test_ticker.py index 07cd414d1609..17eac136cd11 100644 --- a/lib/matplotlib/tests/test_ticker.py +++ b/lib/matplotlib/tests/test_ticker.py @@ -239,6 +239,25 @@ def test_set_params(self): assert loc._base == 4 assert list(loc._subs) == [2.0] + def test_tick_values_correct(self): + ll = mticker.LogLocator(subs=(1, 2, 5)) + test_value = np.array([1.e-01, 2.e-01, 5.e-01, 1.e+00, 2.e+00, 5.e+00, + 1.e+01, 2.e+01, 5.e+01, 1.e+02, 2.e+02, 5.e+02, + 1.e+03, 2.e+03, 5.e+03, 1.e+04, 2.e+04, 5.e+04, + 1.e+05, 2.e+05, 5.e+05, 1.e+06, 2.e+06, 5.e+06, + 1.e+07, 2.e+07, 5.e+07, 1.e+08, 2.e+08, 5.e+08]) + assert_array_equal(ll.tick_values(1, 1e7), test_value) + + def test_tick_values_not_empty(self): + ll = mticker.LogLocator(subs=(1, 2, 5)) + test_value = np.array([1.e-01, 2.e-01, 5.e-01, 1.e+00, 2.e+00, 5.e+00, + 1.e+01, 2.e+01, 5.e+01, 1.e+02, 2.e+02, 5.e+02, + 1.e+03, 2.e+03, 5.e+03, 1.e+04, 2.e+04, 5.e+04, + 1.e+05, 2.e+05, 5.e+05, 1.e+06, 2.e+06, 5.e+06, + 1.e+07, 2.e+07, 5.e+07, 1.e+08, 2.e+08, 5.e+08, + 1.e+09, 2.e+09, 5.e+09]) + assert_array_equal(ll.tick_values(1, 1e8), test_value) + class TestNullLocator: def test_set_params(self): From 1fc559110cff00ba72417deed592ab02106c745f Mon Sep 17 00:00:00 2001 From: haval0 <56519858+haval0@users.noreply.github.com> Date: Wed, 1 Mar 2023 18:38:44 +0100 Subject: [PATCH 2/5] fix: make "bad plot" test fail because fix isn't implemented yet "classic_mode" had to be disabled for the test to successfully replicate the normal behavior of matplotlib. Now the test produces an empty list, which is what should be fixed. --- lib/matplotlib/tests/test_ticker.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/matplotlib/tests/test_ticker.py b/lib/matplotlib/tests/test_ticker.py index 17eac136cd11..1a2ab5b19fe8 100644 --- a/lib/matplotlib/tests/test_ticker.py +++ b/lib/matplotlib/tests/test_ticker.py @@ -249,6 +249,7 @@ def test_tick_values_correct(self): assert_array_equal(ll.tick_values(1, 1e7), test_value) def test_tick_values_not_empty(self): + mpl.rcParams['_internal.classic_mode'] = False ll = mticker.LogLocator(subs=(1, 2, 5)) test_value = np.array([1.e-01, 2.e-01, 5.e-01, 1.e+00, 2.e+00, 5.e+00, 1.e+01, 2.e+01, 5.e+01, 1.e+02, 2.e+02, 5.e+02, From 6d03ed8b653aa6e7da9eec40c3144b11d64e456f Mon Sep 17 00:00:00 2001 From: II-Day-II <74981560+II-Day-II@users.noreply.github.com> Date: Tue, 7 Mar 2023 12:02:57 +0100 Subject: [PATCH 3/5] fix: revert simplification of stride calculation in LogLocator (#3) The attempt to simplify the stride calculation in `a06f343dee3cebf035b74f65ea00b8842af448e9` seems to be the cause of the problem. Using an approach equivalent to what was there before hopefully resolves #24092. --- lib/matplotlib/ticker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/ticker.py b/lib/matplotlib/ticker.py index 9a3f047eb79d..f771ee2cb18d 100644 --- a/lib/matplotlib/ticker.py +++ b/lib/matplotlib/ticker.py @@ -2386,7 +2386,8 @@ def tick_values(self, vmin, vmax): # Get decades between major ticks. stride = (max(math.ceil(numdec / (numticks - 1)), 1) if mpl.rcParams['_internal.classic_mode'] else - (numdec + 1) // numticks + 1) + next(s for s in itertools.count(1) + if numdec // s + 1 <= numticks)) # if we have decided that the stride is as big or bigger than # the range, clip the stride back to the available range - 1 From 503356e9d9cbb9a2225844e2f67f83b9631a99ec Mon Sep 17 00:00:00 2001 From: haval0 <56519858+haval0@users.noreply.github.com> Date: Tue, 7 Mar 2023 14:10:41 +0100 Subject: [PATCH 4/5] fix: make tests slightly error tolerant (#4) uses the assert_almost_equal function instead of assert_array_equal --- lib/matplotlib/tests/test_ticker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/tests/test_ticker.py b/lib/matplotlib/tests/test_ticker.py index 1a2ab5b19fe8..974a0de5c3f4 100644 --- a/lib/matplotlib/tests/test_ticker.py +++ b/lib/matplotlib/tests/test_ticker.py @@ -246,7 +246,7 @@ def test_tick_values_correct(self): 1.e+03, 2.e+03, 5.e+03, 1.e+04, 2.e+04, 5.e+04, 1.e+05, 2.e+05, 5.e+05, 1.e+06, 2.e+06, 5.e+06, 1.e+07, 2.e+07, 5.e+07, 1.e+08, 2.e+08, 5.e+08]) - assert_array_equal(ll.tick_values(1, 1e7), test_value) + assert_almost_equal(ll.tick_values(1, 1e7), test_value) def test_tick_values_not_empty(self): mpl.rcParams['_internal.classic_mode'] = False @@ -257,7 +257,7 @@ def test_tick_values_not_empty(self): 1.e+05, 2.e+05, 5.e+05, 1.e+06, 2.e+06, 5.e+06, 1.e+07, 2.e+07, 5.e+07, 1.e+08, 2.e+08, 5.e+08, 1.e+09, 2.e+09, 5.e+09]) - assert_array_equal(ll.tick_values(1, 1e8), test_value) + assert_almost_equal(ll.tick_values(1, 1e8), test_value) class TestNullLocator: From 874e6ea8610da1f76f886d0327f1e5fc398d771a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Kam=C3=A9us?= Date: Wed, 8 Mar 2023 16:47:48 +0100 Subject: [PATCH 5/5] mnt: simplify stride calculation in LogLocator.tick_values --- lib/matplotlib/ticker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/matplotlib/ticker.py b/lib/matplotlib/ticker.py index f771ee2cb18d..0a685694cabf 100644 --- a/lib/matplotlib/ticker.py +++ b/lib/matplotlib/ticker.py @@ -2386,8 +2386,7 @@ def tick_values(self, vmin, vmax): # Get decades between major ticks. stride = (max(math.ceil(numdec / (numticks - 1)), 1) if mpl.rcParams['_internal.classic_mode'] else - next(s for s in itertools.count(1) - if numdec // s + 1 <= numticks)) + numdec // numticks + 1) # if we have decided that the stride is as big or bigger than # the range, clip the stride back to the available range - 1