Skip to content

[Bug]: LogFormatter minor ticks with minor_thresholds of (0,0) does not behave as documented #25896

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.

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

Open
ksunden opened this issue May 16, 2023 · 2 comments · May be fixed by #26277
Open

[Bug]: LogFormatter minor ticks with minor_thresholds of (0,0) does not behave as documented #25896

ksunden opened this issue May 16, 2023 · 2 comments · May be fixed by #26277

Comments

@ksunden
Copy link
Member

ksunden commented May 16, 2023

Bug summary

In investigating #25894 I discovered that despite being documented as:

To disable labeling of minor ticks when 'labelOnlyBase' is False, use minor_thresholds=(0, 0). This is the default for the "classic" style.

It actually only removes some minor tick labels (e.g. with base of 10: 11-14 will get labels, but 16-19 (15 is edge case, dependent on floating point rounding, 1.4, or 140 also similar))

Code for reproduction

import matplotlib.pyplot as plt
import matplotlib.ticker as mticker

fig, ax = plt.subplots()
ax.set_yscale("log")

formatter = mticker.LogFormatter(minor_thresholds=(0,0))
ax.yaxis.set_minor_formatter(formatter)

ax.set_ylim(10, 20)
plt.show()

Actual outcome

Figure_2

formatter(10) # '10'
formatter(11) # '11'
formatter(12) # '12'
formatter(13) # '13'
formatter(14) # '14'
formatter(15) # '15'
formatter(16) # ''

Expected outcome

Figure_2

formatter(10) # ''
formatter(11) # ''
formatter(12) # ''
formatter(13) # ''
formatter(14) # ''
formatter(15) # ''
formatter(16) # ''

Additional information

Only really affects plots that are zoomed in to much less than one decade, which arguably doesn't make sense to use a log scale, but still seems incongruous with the documented "this turns off minor ticks".

Has to do with when coeff rounds down to 1 in:

fx = math.log(x) / math.log(b)
is_x_decade = _is_close_to_int(fx)
exponent = round(fx) if is_x_decade else np.floor(fx)
coeff = round(b ** (fx - exponent))
if self.labelOnlyBase and not is_x_decade:
return ''
if self._sublabels is not None and coeff not in self._sublabels:
return ''

I'm not quite sure what the proper solution is, perhaps making self._sublabels=set() instead?

Operating system

Linux

Matplotlib Version

3.8 (main)

Matplotlib Backend

No response

Python version

3.11

Jupyter version

No response

Installation

git checkout

@rcomer rcomer linked a pull request Mar 3, 2025 that will close this issue
5 tasks
@rcomer rcomer removed a link to a pull request Mar 6, 2025
5 tasks
@anntzer
Copy link
Contributor

anntzer commented Mar 6, 2025

I think the fix here is

diff --git i/lib/matplotlib/ticker.py w/lib/matplotlib/ticker.py
index 4c33e8c9af..5cb1ca6746 100644
--- i/lib/matplotlib/ticker.py
+++ w/lib/matplotlib/ticker.py
@@ -964,7 +964,7 @@ class LogFormatter(Formatter):
             # It's probably a colorbar with
             # a format kwarg setting a LogFormatter in the manner
             # that worked with 1.5.x, but that doesn't work now.
-            self._sublabels = {1}  # label powers of base
+            self._sublabels = np.array([1])  # label powers of base
             return
 
         b = self._base
@@ -995,17 +995,16 @@ class LogFormatter(Formatter):
 
         if numticks > self.minor_thresholds[0]:
             # Label only bases
-            self._sublabels = {1}
+            self._sublabels = np.array([1])
         elif numdec > self.minor_thresholds[1]:
             # Add labels between bases at log-spaced coefficients;
             # include base powers in case the locations include
             # "major" and "minor" points, as in colorbar.
-            c = np.geomspace(1, b, int(b)//2 + 1)
-            self._sublabels = set(np.round(c))
+            self._sublabels = np.geomspace(1, b, int(b)//2 + 1).round()
             # For base 10, this yields (1, 2, 3, 4, 6, 10).
         else:
             # Label all integer multiples of base**n.
-            self._sublabels = set(np.arange(1, b + 1))
+            self._sublabels = np.arange(1, b + 1)
 
     def _num_to_string(self, x, vmin, vmax):
         return self._pprint_val(x, vmax - vmin) if 1 <= x <= 10000 else f"{x:1.0e}"
@@ -1021,11 +1020,10 @@ class LogFormatter(Formatter):
         fx = math.log(x) / math.log(b)
         is_x_decade = _is_close_to_int(fx)
         exponent = round(fx) if is_x_decade else np.floor(fx)
-        coeff = round(b ** (fx - exponent))
 
         if self.labelOnlyBase and not is_x_decade:
             return ''
-        if self._sublabels is not None and coeff not in self._sublabels:
+        if self._sublabels is not None and not np.isin(x, self._sublabels * b**exponent):
             return ''
 
         vmin, vmax = self.axis.get_view_interval()

i.e. to detect whether if a minor tick is at a (sublabel * b**expoenent value) (e.g. 2*10^1 = 20), just compute all these values, rather than trying to round() the mantissa (this is wrong if we have subticks coming from an AutoLocator in the case of #12865.

Tests & PR left as an exercise.

@anntzer
Copy link
Contributor

anntzer commented Mar 11, 2025

Actually the fix above is likely wrong because it would break #12865 (we actually do want these intermediate ticks to be labeled). I'm not entirely sure what the clean fix would be, but the patch at #29698 (comment) still seems reasonable to me (even though it seems to be mostly working around the underlying issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants