Fix MultipleLocator proposed by @anntzer #7123
Conversation
| locs = vmin - base + np.arange(n + 3) * base | ||
| nmin = round(self._base.ge(vmin) / base) | ||
| nmax = round(self._base.le(vmax) / base) | ||
| locs = np.arange(nmin, nmax + 1) * base |
There was a problem hiding this comment.
Why can't this code be simplified to: `np.arange(vmin, vmax + base, base)? Floating point errors?
There was a problem hiding this comment.
Sorry… np.arange(self._base.ge(vmin), vmax + base, base) would be the correct solution.
There was a problem hiding this comment.
Your proposal can include a value that's above vmax (eg. vmin=.5, vmax=2.5, base=1 will return [1, 2, 3]). I guess another solution that's safe against floating point errors is
np.arange(self._base.ge(vmin), self._base.le(vmax) + base / 2, base)
(the + base / 2 should take care of any fpe).
There was a problem hiding this comment.
Yep: I am just concern about the tests that don't make sense to me.
There was a problem hiding this comment.
I had a quick look and am pretty sure that the test values are wrong (i.e. they were picked to match the previous algorithm, which returns out of bounds values; they should be changed to match the new algorithm).
|
I have done a couple of tests locally, and I have to admit that the current tests seems obscure… |
|
I think that this ties back to the other issue with assuming all of the ticks are in view limits (#6647) and what layer of the stack is responsible for the final filtering. |
|
When this is cleared up please clean up the figure added in #7084 |
|
This is pretty stale, so I'm assuming this change isn't urgently required by anyone, but feel free to reopen if wanted... |
This is the fix proposed by @anntzer in #7122 related to a bug in MultipleLocator ticker.