None __ne__ behavior changes in 3.12

[andrewluotechnologies]

Bug report

Bug description:

Python 3.11.6 (tags/v3.11.6:8b6ee5b, Oct  2 2023, 14:57:12) [MSC v.1935 64 bit (AMD64)] on win32
>>> None.__ne__(None)
False

Python 3.12.0 (tags/v3.12.0:0fb18b0, Oct  2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32
>>> None.__ne__(None)
NotImplemented

In 3.11, None.__ne__(None) returned False but in 3.12 it returns NotImplemented. This is a breaking behavior change that is either a bug or should be documented.

CPython versions tested on:

3.11, 3.12

Operating systems tested on:

Windows

Linked PRs

• gh-112125: Fix None.__ne__(None) returning NotImplemented instead of False #112504
• [3.12] gh-112125: Fix None.__ne__(None) returning NotImplemented instead of … #112827

[ericvsmith]

What's the scenario where you're calling __ne__ instead of using !=? Also, operator.ne(None, None) works as expected.

The recommendation is to not call dunder methods directly.

[zware]

Out of curiosity, I tracked this down to commit 432117c, which just gave None a constant hash value. I think it's rather a stretch to consider this to be anything but an implementation detail changing, and we'll need to see some compelling evidence to change or document this effect.

[andrewluotechnologies]

I ran into this issue with code such as filter(None.__ne__, iterable). I also did some searching on Google and Github and I do see this pattern elsewhere also: https://github.com/search?q=None.__ne__&type=code

[pochmann]

rhettinger also showed that use case / idiom previously:

https://stackoverflow.com/revisions/16097112/1

#79893 (comment)

[andrewluotechnologies]

The list comprehension requires materializing the result in memory, but if we want to say that using None.__ne__ in this was is not recommended, a more direct replacement (without materializing the result in memory) is probably filter(lambda x: x is not None, iterable). But my concern here is primarily that we should document this change if it is intentional given that this pattern is not unheard of.

[rhettinger]

Out of curiosity, I tracked this down to commit 432117c, which just gave None a constant hash value.

At first, I couldn't see how this edit would have caused a visible behavior change. I think what is happening is that we used to inherit object_richcompare, but when object.__hash__ is overridden, the PyType_Ready logic substitutes more generic equality logic when creating the __eq__ slot wrapper. This change almost certainly wasn't intended. Certainly, a NotImplemented result makes no sense at all when a comparison has the same type on both sides.

I would like to see the tp_richcompare slot for NoneType explicitly set to object_richcompare. This will fix an oversight in the prior edit, get back to the formerly fast code for identity comparison, and restore the previous semantics.

The goal of the previous edit was simply to make the hash value constant. The code review did not detect the downstream effect on __eq__. We should fix this.

[zyv]

We also got hit by this pretty bad, using filter(None.__ne__, iterables) everywhere.

Just to clarify, @rhettinger, is this a canonical recommended idiom and we've just got hit by a bus, or it's a cool hack that's bound to fail randomly and the "supported" idiom would be filter(partial(operator.ne, None), xs), so we should do a mass SAR operation on our code base ASAP?

[pochmann]

@zyv In the latter case you might want to use operator.is_not instead. (Or use a generator expression also using is not None.) Likely you've used None.__ne__ because that was available and worked, not because it's really what you want.

[rhettinger]

@zyv The comparison None.__ne__(None) is supposed to return False. So we're about to fix it. If no one steps up, I will do it.

While it is permissible (and fast) to call the dunder methods in code like filter(None.__ne__, iterables), there is a risk that the code does isn't doing exactly what people expect it to do. Comparison operators have more logic than just calling a single dunder method. If the dunder method is called directly, the cross-type comparison logic is lost.

If instead you write filter(functools.partial(operator.ne, None), iterables), the test will always give the same result as None != element. That might matter in an exotic case with objects that override __eq__ to report being equal to None. That is highly unlikely. What you most likely looking for is an equivalent to element is not None. As @pochmann suggested, operator.is_not would be the best choice.

[vstinner]

I merged your PR, thank you @andrewluotechnologies. I backported the change manually to 3.12 to fix a merge conflict.