Skip to content

FIX: LogFormatter minor ticks with minor_thresholds of (0,0) misbehaves #26277

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Rylie-W
Copy link

@Rylie-W Rylie-W commented Jul 8, 2023

PR summary

PR checklist

@oscargus
Copy link
Member

oscargus commented Jul 9, 2023

(Deleted my comment since I realized that it can be both an empty set and None.)

Seems like this breaks several other things.

@Rylie-W Rylie-W marked this pull request as draft July 9, 2023 20:52
@Rylie-W Rylie-W force-pushed the bugfix-for-issue-25896 branch from 3f120f7 to 13b8af4 Compare July 10, 2023 06:37
@Rylie-W Rylie-W force-pushed the bugfix-for-issue-25896 branch from 13b8af4 to e06b857 Compare July 10, 2023 17:39
@Rylie-W
Copy link
Author

Rylie-W commented Jul 10, 2023

I'm not sure which fix is better, or if there is any other superior solution available. Based on the commit history, there are two potential fixes:

  1. alternative fix. The fix produces the expected outcomeand aligns with the description in the doc:

If labelOnlyBase is False, these two numbers control the labeling of ticks that are not at integer powers of base; normally these are the minor ticks. The controlling parameter is the log of the axis data range. In the typical case where base is 10 it is the number of decades spanned by the axis, so we can call it 'numdec'. If numdec <= all, all minor ticks will be labeled. If all < numdec <= subset, then only a subset of minor ticks will be labeled, so as to avoid crowding. If numdec > subset then no minor ticks will be labeled.
...
To disable labeling of minor ticks when 'labelOnlyBase' is False, use minor_thresholds=(0, 0). This is the default for the "classic" style.

Note that it assumes labelOnlyBase is True for all the major formatters while False for minor formatters, which could contradict with the doc stating:

This is normally True for major ticks and False for minor ticks.

What about "abnormally"? Can a major formatter have 'labelOnlyBase' set to False? Is there an exceptional case?

  1. FIX: LogFormatter minor ticks with minor_thresholds of (0,0) misbehaves. It uses the same minor_thresholds logic for both major and minor formatter, and allows a major formatter with labelOnlyBase=False or a minor formatter with labelOnlyBase=True. But it contradicts with the following description in the documentation:

If labelOnlyBase is False, these two numbers control the labeling of ticks that are not at integer powers of base; normally these are the minor ticks. The controlling parameter is the log of the axis data range. In the typical case where base is 10 it is the number of decades spanned by the axis, so we can call it 'numdec'. If numdec <= all, all minor ticks will be labeled. If all < numdec <= subset, then only a subset of minor ticks will be labeled, so as to avoid crowding. If numdec > subset then no minor ticks will be labeled.

When minor_thresholds is set to (0, 0), namely, numdec > subset, a formatter labels bases only whether labelOnlyBase is True or not. In the example from issue #25896, its outcome is as follows:

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)

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

Furthermore, what does the term "classic" style refer to in the documentation? Maybe we could provide a more precise description.

@Rylie-W Rylie-W marked this pull request as ready for review July 11, 2023 17:05
@oscargus
Copy link
Member

Classic is the pre-2.0 default style.

fix test failures

alternative fix

fix format

remove irrelevant code

remove label_expected
@Rylie-W Rylie-W force-pushed the bugfix-for-issue-25896 branch from 29b913a to 6e9179d Compare July 22, 2023 12:11
@Rylie-W
Copy link
Author

Rylie-W commented Jul 23, 2023

Hi @oscargus, could you review the PR again?

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

Successfully merging this pull request may close these issues.

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