FIX: make sure we have more than 1 tick with small log ranges #18754
Conversation
If we have both a small target number of ticks and a small displayed range we would previously pick a tick stride that was the same size or bigger than the visible data range which would result in only 1 tick being visible on the axis. This patch ensures that, with a floor at 1, the stride is smaller than the estimated data range. re-closes matplotlib#8768 (same symptoms, different cause)
| @@ -2479,6 +2479,13 @@ def tick_values(self, vmin, vmax): | |||
| if mpl.rcParams['_internal.classic_mode'] else | |||
| (numdec + 1) // numticks + 1) | |||
|
|
|||
| # if we have decided that the stride is as big or bigger than | |||
There was a problem hiding this comment.
Is this really the right solution to the original problem? To me, it seems that numticks is 1 because there isn't room for more ticks. Do we really want ticks that overwrite each other for very narrow axes?
There was a problem hiding this comment.
If you are very narrow, there's no good solution. Either you have only one tick, which makes your scale uninterpretable. Or you have overlap. 🤷
There was a problem hiding this comment.
Sorry, let me rephrase: We clip numticks to 2, so why don't we have 2 ticks?
There was a problem hiding this comment.
numticks is an upper bound and only "best effort" at that.
The issue is that based on numtick and a rounded "in" version of the range we pick a stride between major ticks (which is meant to catch things like 30 orders of magnitude with 9 ticks). If we propose 3 ticks, have a range of [5, 150], then we "round out" to [1, 100] for the range giving us numdec = 2 and numticks = 3 and a stride of (3 / 3) + 1 == 2 which means we put ticks at (10 ^ {-2, 0, 2, 4}) which in turn means that only the tick at 100 is actually shown. Additionally because we only show the minor ticks if the major stride is equal to 1 you end up with exactly 1 tick and 1 label shown even though the users / auto logic asked for up to 3.
I think that this fix (which fixes the values after doing a bunch of heuristics) is a better and more reliable fix than making the heuristics more complicated.
Mostly. The issue I described can be pushed up a bit to hit the case where we should have ~2 decades in view and we pick a 3 decade stride and in that case we still don't have the minor ticks, but will at least get 2 major major ticks. I did some manually testing with this by making the y-axis super small and then interactively panning / zooming to see if anything bad looking happened... |
… with small log ranges
…754-on-v3.3.x Backport PR #18754 on branch v3.3.x (FIX: make sure we have more than 1 tick with small log ranges)
| ll.set_params(numticks=numticks) | ||
| for top in [5, 7, 9, 11, 15, 50, 100, 1000]: | ||
| ticks = ll.tick_values(.5, top) | ||
| assert (np.diff(np.log10(ll.tick_values(6, 150))) == 1).all() |
There was a problem hiding this comment.
@tacaswell There seems to be a clear typo in the test here: you don't do anything with ticks, and the assert is the same on each loop iteration. Do you remember what you intended to do?
PR Summary
If we have both a small target number of ticks and a small displayed
range we would previously pick a tick stride that was the same size or
bigger than the visible data range which would result in only 1 tick
being visible on the axis.
This patch ensures that, with a floor at 1, the stride is smaller than
the estimated data range.
re-closes #8768 (same symptoms, different cause)
attn @aggna
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsandpydocstyle<4and runflake8 --docstring-convention=all).