Thanks, I think that this makes sense! As seen, we usenp.floor, just afterround in the second failure. Although it may just be a coincidence.

Can you please add a test for this? I think a smoke test, where you basically plot something (simple) that currently triggers the error should be enough. With a comment that it is a smoke test and point to this PR for motivation. In this way we can make sure that the error doesn't return later if someone thinks "well, I can replace that np.round with round"...

One can of course discuss if actually all round should be np.round, like now, or if one should read the code a bit more careful and see if any can stay. On the other hand, one can also question if round is actually that much better for any reason? It is not so obvious from the code why sometimes round is chosen and sometimes np.round (except when we know that there is a vector involved).

Hey so I tried to reproduce the issue I experienced, with somenp.inf data, and I didn't succeed. I also failed to retrieve the data that I had back then. I still think it is worth discussing with other developers, but I will leave my effort on this for now.

In principle I agree this makes sense, but there might be some other changes needed (likenp.round returns floats where asround returns ints so we might need extra casts).

You can reproduce this with

importmatplotlib.pyplotaspltplt.ion()fig,ax=plt.subplots()importnumpyasnpax.loglog(np.logspace(0,10,10),np.logspace(0,10,10)**2)ax.set_yticks([1,10,np.inf])

which suggest that we should be doing better validation on the way intoFixedLocator. Put another way, if we are gettingnp.inf here then there is very likely a bug (or user error) someplace else that we should deal with instead.

Wow thanks for creating this code example! I will put something similar in the tests I will hopefully write after discussing a solution.

Put another way, if we are gettingnp.inf here then there is very likely a bug (or user error) someplace else that we should deal with instead.

Definitely - havingnp.inf in your data is not scientific :), and perhaps a good explanatory warning should be printed when this is attempted. The minimum expectation is thatmatplotlib won't crash when it encounters such a situation.

Havingnp.inf in thedata should be OK (we take care of this a bunch of places), it is asking for a tick atnp.inf that is the problem.

OK I have an issue with running the tests locally, so I hope it will be OK with you that I will use the PR's CI to do that. Now I pushed a new test, without the fix yet, expecting to see the only test I added failing, and I will push a fix afterwards, expecting the issue to be fixed with it.

OK tests failures from CI:

=================================== FAILURES ===================================_____________________________ test_yticks_with_inf _____________________________[gw2] darwin -- Python 3.12.3 /Library/Frameworks/Python.framework/Versions/3.12/bin/python    def test_yticks_with_inf():        fig, ax = plt.subplots()        ax.loglog(np.logspace(0, 10, 10), np.logspace(0, 10, 10)**2)        ax.set_yticks([1, 10, np.inf])>       fig.draw_without_rendering()lib/matplotlib/tests/test_ticker.py:1837: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ lib/matplotlib/figure.py:3171: in draw_without_rendering    self.draw(renderer)lib/matplotlib/artist.py:95: in draw_wrapper    result = draw(artist, renderer, *args, **kwargs)lib/matplotlib/artist.py:72: in draw_wrapper    return draw(artist, renderer)lib/matplotlib/figure.py:3155: in draw    mimage._draw_list_compositing_images(lib/matplotlib/image.py:132: in _draw_list_compositing_images    a.draw(renderer)lib/matplotlib/artist.py:72: in draw_wrapper    return draw(artist, renderer)lib/matplotlib/axes/_base.py:3113: in draw    mimage._draw_list_compositing_images(lib/matplotlib/image.py:132: in _draw_list_compositing_images    a.draw(renderer)lib/matplotlib/artist.py:72: in draw_wrapper    return draw(artist, renderer)lib/matplotlib/axis.py:1422: in draw    ticks_to_draw = self._update_ticks()lib/matplotlib/axis.py:1300: in _update_ticks    major_labels = self.major.formatter.format_ticks(major_locs)lib/matplotlib/ticker.py:217: in format_ticks    return [self(value, i) for i, value in enumerate(values)]lib/matplotlib/ticker.py:1088: in __call__    is_x_decade = _is_close_to_int(fx)_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ x = inf    def _is_close_to_int(x):>       return math.isclose(x, round(x))E       OverflowError: cannot convert float infinity to integerlib/matplotlib/ticker.py:2265: OverflowError------------------------------ Captured log call -------------------------------

Now pushed the suggested fix. Hopefully CI will be green 🤞 .

Aish there are more errors then I anticipated... What if I will replace allmath. functions with theirnumpy equivalent? Not onlynp.round.. Would that be accepted?

🎉 Managed to finally fix the new test I added (took me a while because ofthis). CI is green now. Your review is welcome.

BTW@tacaswell CI is green here as well for a while now :).

Can you try rebasing to see if we have a fix for the appevoyer failure on main?

Does this only letnp.inf pass through as a value or does it also sort out/fix why a locator suggested putting a tick at inf?

Does this only letnp.inf pass through as a value or does it also sort out/fix why a locator suggested putting a tick at inf?

I'm not sure I understood the 2nd part - what do you mean by "tick at inf"? If you are asking in general about the motivation for the PR, it is a bit hard for me to find the most accurate words to explain it, but for sure it fixes the test added in the 1st commit, and I believe it fixes many other potential issues with nan and infinite values.

I'm a bit hesitant to change all the math <-> np calls. This may have surprising side-effects. How much of this is to handleinf inset_yticks? I'd rather error there and keep other changes minimal.

In the original code that produced the issue I encountered and tried to fix here, I didn't explicitly usedset_yticks with annp.inf in the argument, something internal inticker.py made it try to set these yticks for the set of y values I had in my code. It is indeed unfortunate that I am not able now to get a list of such y values, but I can imagine it might happen given the usage oflog to figure out where to put yticks.

Are you able to produce a minimal example of the issue we are trying to fix here? Without that, we're basically blind:

• we cannot judge whether the PR is the appropriate fix or whether another approach would be better
• we cannot add tests to ensure that the fixed behavior is not unintentionally reverted through future code changes

The proposed math <-> np change, is substantial (in particular because the changes are subtle). We'd need a very informed decision to go that direction. One prerequisite is that we exactly know what problem we are fixing with that.

Are you able to produce a minimal example of the issue we are trying to fix here?

I agree that the test I added here (suggested by@tacaswell inthis comment) might not be very satisfying because it is a bit superficial, so I'm happy to report I did manage now to create a less superficial MWE :):

importmatplotlib.pyplotaspltfig,ax=plt.subplots()ax.plot([1,2], [1,1e261])ax.set_yscale("log")plt.show()

Givesmath.isclose(x, round(x)) error right? Note how if you change the261 to260 you get no error. Another interesting thing to try is the following:

#!/usr/bin/env pythonimportnumpyasnpimportmathforoominrange(300,4400):x=np.float128(10**oom)print("oom(x) = ",oom,np.log(x),math.log(x))

The above prints inf in the last word afteroom = 309, andnp.float128 fails afteroom = 4300.

If I usenp.float64 (Numpy's default float), it fails whenoom = 309, exactly like the Python nativefloat function.

Without that, we're basically blind:

• we cannot judge whether the PR is the appropriate fix or whether another approach would be better
• we cannot add tests to ensure that the fixed behavior is not unintentionally reverted through future code changes

The proposed math <-> np change, is substantial (in particular because the changes are subtle). We'd need a very informed decision to go that direction. One prerequisite is that we exactly know what problem we are fixing with that.

I agree overall with the development workflow you try to impose, but I find it a bit peculiar to justify the current state in whichnumpy/math functions are chosen arbitrarily (at least it seems like so). The PR simply suggests to be consistent, which is something I thought would be accepted even without a test, simply because it makes sense.

I can replace the test I added with the above MWE and verify that the PR fixes the test, but I want to know first what is the direction you'd like to take this now that I managed to obtain further clues as for the bug I experienced.

but I find it a bit peculiar to justify the current state in which numpy/math functions are chosen arbitrarily (at least it seems like so). The PR simply suggests to be consistent, which is something I thought would be accepted even without a test, simply because it makes sense.

Unfortunately, it's not that simple:

• There's no clear right or wrong for using math vs. numpy. The behavior is subtly different (which you actually rely on for making your case work). But we have no insight whether the changed behavior has undesired effects in other cases.
• Therefore, I'd try to minimize changes (if it ain't broken, don't fix it), which needs a clear insight why the current code is not suitable. -> Understand the problem and exactly solve that.

I can replace the test I added with the above MWE and verify that the PR fixes the test, but I want to know first what is the direction you'd like to take this now that I managed to obtain further clues as for the bug I experienced.

The next step is to follow the MWE code through a debugger and find out where the infinity comes in. There must be a place where 1e261 is turned into infinity, and most likely that's the code we have to fix.

Edit: Part of the issue is that we add whitespace around the data (aka.margin) so that data points are not at the edge of the Axes. On a log scale, that can be a significant numerical amount. To take that effect out, change the example to:

import matplotlib.pyplot as pltfig, ax = plt.subplots()ax.plot([1,2], [1, 1e269])ax.set_yscale("log")ax.margins(0)plt.show()

which still works, but will fail for 1e270. Or even better, don't bother with data but isolate the topics to the axis limits

import matplotlib.pyplot as pltfig, ax = plt.subplots()ax.set_ylim(1, 1e270)  # works with 1e269ax.set_yscale("log")plt.show()

The next step is to follow the MWE code through a debugger and find out where the infinity comes in. There must be a place where 1e261 is turned into infinity, and most likely that's the code we have to fix.

OK, that's sort of what I did in the beginning, although I didn't have such a good MWE. I also described that process in the very first comment of this PR. In the meantime I reordered the commits a bit, and added a test that currently fails, as I have not yet found an optimal solution for it. Your view on this now is most welcome.

Given the recent discussion I'm going to move this to the 3.11 milestone.

The actual MWE is

from matplotlib.ticker import LogLocatorLogLocator().tick_values(0, 1e270)

and the relevant code:

decades is[-31 0 31 62 93 124 155 186 217 248 279 310] and withb=10.0 we end up on
ticklocs[1.e-031 1.e+000 1.e+031 1.e+062 1.e+093 1.e+124 1.e+155 1.e+186 1.e+217 1.e+248 1.e+279 inf]

with numpy 10.0 ** 310 == inf (pure python throws an OverflowError instead). Note that the largest number you can represent with 64-bit floats is ~ 1.8e308.

A tick locator should never place ticks at inf. The proper solution to this problem is to tune the tick location algorithm inLogLocator.tick_values().

The actual MWE is

from matplotlib.ticker import LogLocatorLogLocator().tick_values(0, 1e270)

and the relevant code:

matplotlib/lib/matplotlib/ticker.py

Lines 2463 to 2464 in4468488

	decades=np.arange(math.floor(log_vmin)-stride,
	math.ceil(log_vmax)+2*stride,stride)

matplotlib/lib/matplotlib/ticker.py

Line 2473 in4468488

ticklocs=b**decades

decades is[-31 0 31 62 93 124 155 186 217 248 279 310] and withb=10.0 we end up onticklocs[1.e-031 1.e+000 1.e+031 1.e+062 1.e+093 1.e+124 1.e+155 1.e+186 1.e+217 1.e+248 1.e+279 inf]

with numpy 10.0 ** 310 == inf (pure python throws an OverflowError instead). Note that the largest number you can represent with 64-bit floats is ~ 1.8e308.

A tick locator should never place ticks at inf. The proper solution to this problem is to tune the tick location algorithm inLogLocator.tick_values().

That's true, and it'd indeed be nice to start with writing such a test specifically forLogLocator, but even if that would be fixed, it is not clear to me what would be the next step. I tried:

diff --git i/lib/matplotlib/ticker.py w/lib/matplotlib/ticker.pyindex 63a519f793..74cfac52e3 100644--- i/lib/matplotlib/ticker.py+++ w/lib/matplotlib/ticker.py@@ -2401,7 +2401,7 @@ class LogLocator(Locator):         have_subs = len(subs) > 1 or (len(subs) == 1 and subs[0] != 1.0)          decades = np.arange(math.floor(log_vmin) - stride,-                            math.ceil(log_vmax) + 2 * stride, stride)+                            math.ceil(log_vmax) + 2 * stride, stride, dtype=np.float128)          if have_subs:             if stride == 1:

But then I encountered an issue inLogFormatterSciNotation, here:

Which makesfx beinf. That is also fixable by:

diff --git i/lib/matplotlib/ticker.py w/lib/matplotlib/ticker.pyindex 74cfac52e3..5f3222fc1f 100644--- i/lib/matplotlib/ticker.py+++ w/lib/matplotlib/ticker.py@@ -1088,7 +1088,7 @@ class LogFormatterMathtext(LogFormatter):         b = self._base          # only label the decades-        fx = math.log(x) / math.log(b)+        fx = np.log(x) / np.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))

And then the originaltest_LogFormatter_almost_inf passes. However, it is not that satisfying, as I'm not capable of explaining up to what number willnp.float128 be good enough. And also equally importantly, I don't know how to testLogLocator'sdecades calculation without changing too many inner parts there. Not only that, if I use1e400 intest_LogFormatter_almost_inf, it won't fail but because 1e400 is already treated asinf, and it shouldn't, althoughnp.float128 is used whendecades are computed...

This rabbit hole is what made me use numpy's functions all the time, as they seem a bit more accurate almost all of the times. If we'll after all still go in such approach, it'd should be a good idea to use float128 all the time.

Rewriting for float128 would be a non-trivial medium-sized task. Most users don't use float 128 anyway and introducing it only to have larger ticklabels feels overkill - or at least a completely separate discussion with much larger impact.

I'd rather try and manipulate the ticker so that it does not produce tick label positions beyond 1e308. The very naive approach would be to clip the nan position. One has to try whether that looks ok. Otherwise, you'd need to come up with a better decades algorithm for the edge case, e.g. choose something between vmax and 3e8 as the top tick position and a suitable spacing going down in regular decades.

Apparently the maximal float size is actually1.7976931348623157e+308, and can be computed like this:

importsysimportnumpyasnpprint(np.finfo(np.float64).max)print(sys.float_info.max)

Rewriting for float128 would be a non-trivial medium-sized task. Most users don't use float 128 anyway and introducing it only to have larger ticklabels feels overkill - or at least a completely separate discussion with much larger impact.

I think, that the natural thing to do would be to inherit thedtype (float64 orfloat128) of the user's input. If they usefloat128 because they know they have very large floats, we shouldn't disallow them to do so. If they use integers OTH (which can grow infinitely large), we should disallow them to use integers bigger then the maximum offloat128. Perhaps some of this behavior should be handled at the level ofLocator.

Indeed that's a non-trivial task though, so I'm not sure I'd be able to invest time in it. TBH, the issue I encountered here is not an active issue for me anymore. I just kept giving this attention because I want to contribute back to Matplotlib. I think that the 3 commits prior to the last one are worth merging either way, and we can create a separate issue with all the details we have aggregated in the last few days.

I'd rather try and manipulate the ticker so that it does not produce tick label positions beyond 1e308.

I think that silently ignoring such inputs can be confusing for the users, so I'd raise an error much earlier.

I think, that the natural thing to do would be to inherit the dtype (float64 or float128) of the user's input.

AFAIK, we don't have any guarantees on that, and it would be a massive undertaking. Basically, you'd have to make sure that all internal calculations do respect this. I don't think we can or should go there in the foreseeable future.

I'd rather try and manipulate the ticker so that it does not produce tick label positions beyond 1e308.

I think that silently ignoring such inputs can be confusing for the users, so I'd raise an error much earlier.

I'm not ignoring inputs. The inputs are the data range. The TickLocator tries to invent ticks at nice positions. And results incurrent ticks in below image. The naive solution would be to clip the invented right-most tick. The axis limits would still cover the whole data. The proposed alternative dcades algorithm could try to find the solution foralternative ticks.