GeezerVison

I'm not familiar with floating-point behavior, but from the linked numpy issue it seems there is an arguable, obscure CPython bug. Could you open an issue so we can discuss whether CPython needs to change, and add tests for the behavior that we should want?

Hmm. There's some history here. The avoidance of Py_NaN and Py_HUGE_VAL was deliberate, at least in part to avoid operations that might set floating-point exceptions.

I don't think there's any need to change the infinity-handling code here; that bit pattern should be correct regardless of machine; could we stick to just looking at the NaN operations?

One other thing: Py_NaN will not necessarily have the same sign as the value computed from the bit pattern, and I'd expect people to notice and complain if the sign bit changes (arguably they shouldn't, but that's a different discussion). I'd prefer to be in control of the sign (and the payload) here, and the Py_NaN macro doesn't give us that control.

Instead of this approach, could we maybe introduce a preprocessor #define that specifies how the parity of bit 51 relates to quiet / signalling, and use that in constructing the quiet NaN from a bit pattern?

Gah. Bit 51, not bit 52; edited the above text to correct.

🤖 New build scheduled with the buildbot fleet by @mdickinson for commit fd39437 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

FTR, my comment above is out of date; see the linked issue for discussion.

@seberg I've started a buildbot run. I'm happy with the shape of this PR if the buildbots agree.

We're missing a news entry; do you want to add one, or shall I? (I guess most of this could be classed as internal refactoring, which doesn't really need a news entry, but the MIPS fix probably deserves something.)

Happy either way, you probably got a more precise angle on what to write.

The Py_INFINITY definition may be the most interesting part for a news entry (but the MIPS fix doesn't hurt).

Updated to add a news entry and fix another couple of Py_HUGE_VAL instances. I'll start another buildbot run, and merge if the buildbots look happy. @seberg Thank you for the PR!

🤖 New build scheduled with the buildbot fleet by @mdickinson for commit fabf7d3 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

🤖 New build scheduled with the buildbot fleet by @mdickinson for commit e8c36a2 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

Okay, we're getting relevant buildbot failures. E.g., from here

======================================================================
FAIL: test_nan_signs (test.test_float.InfNanTest.test_nan_signs)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\buildarea\pull_request.bolen-windows10\build\Lib\test\test_float.py", line 1045, in test_nan_signs
    self.assertEqual(copysign(1.0, float('nan')), 1.0)
AssertionError: -1.0 != 1.0
----------------------------------------------------------------------

Looks like we will need that fabs call around the NAN macro (which isn't really surprising - there's nothing in the C standard that says that NAN should have any particular sign bit).

The question is whether we want to put the fabs in just for the float("nan") and math.nan uses (and complex equivalents) and leave Py_NAN as the "system" NaN, or include the fabs call in the Py_NAN macro.

Hmmmpf, I underestimated the complexity... Would be tempted to include it in the macro but don't have a real opinion or gut feeling on it (assuming any reasonable compiler optimizes it well).
The reason is that there are three places where it matters probably and 2 of them use -Py_NAN (all other places are probably errors anyway). OTOH, of course in typical use it shouldn't matter at all.

(NumPy has a similar use of -NPY_NAN although not sure it matters. Actually NumPy currently still hard-codes the NANf value and casts it. Somehow the cast from float to double apparently "fixes" the bit pattern on MIPS...)

I'm leaning towards leaving Py_NAN as a simple wrapper around NAN (or __builtin_nan) and only adding the fabs where we need it - i.e., for math.nan and float("nan").

My general philosophy on this is that most of the time we shouldn't care too much about signs of NaNs. float("nan") is an exception because it seems like too much of a violation of the POLS to have float("nan") give a negative NaN while float("-nan") gives a positive NaN. For math.nan, in the absence of backwards compatibility constraints I think I'd actually prefer math.nan just to return exactly what C's NAN does, but I also think that that would be an unacceptable backwards compatibility break, so we're stuck with forcing a positive NaN in this case.

FWIW, I believe that one time that people end up caring about signs of Python NaNs is when the NaN in question hits a C-level printf (in some extension module) and the printf implementation in question likes to put negative signs on negative NaNs. (The C standard doesn't seem to have anything to say about what printf should do here, and actual implementations seem to differ.)

🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit a35ae19 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

Thanks for the updates; that matches what I had in mind. Watching the buildbots (thanks @JelleZijlstra for triggering) ...

Buildbots are happy, and so am I. Merging. Thank you, @seberg!

I'll try to find time shortly (though not today) to replace remaining occurrences of Py_HUGE_VAL with Py_INFINITY (at least, in those places where infinity is clearly what's intended, which I suspect is all of them).