You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
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
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.
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.
Reacted by Jelle Zijlstra, Alex Waygood and Edgar Ramírez Mondragón
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?
@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.
@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.
Activity
ericvsmith commentedon Nov 16, 2023
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 commentedon Nov 16, 2023
Out of curiosity, I tracked this down to commit 432117c, which just gave
Nonea 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 commentedon Nov 20, 2023
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=codepochmann commentedon Nov 20, 2023
rhettinger also showed that use case / idiom previously:
https://stackoverflow.com/revisions/16097112/1
#79893 (comment)
andrewluotechnologies commentedon Nov 20, 2023
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 probablyfilter(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 commentedon Nov 23, 2023
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 whenobject.__hash__is overridden, thePyType_Readylogic substitutes more generic equality logic when creating the__eq__slot wrapper. This change almost certainly wasn't intended. Certainly, aNotImplementedresult makes no sense at all when a comparison has the same type on both sides.I would like to see the
tp_richcompareslot forNoneTypeexplicitly set toobject_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 commentedon Nov 24, 2023
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 commentedon Nov 24, 2023
@zyv In the latter case you might want to use
operator.is_notinstead. (Or use a generator expression also usingis not None.) Likely you've usedNone.__ne__because that was available and worked, not because it's really what you want.rhettinger commentedon Nov 25, 2023
@zyv The comparison
None.__ne__(None)is supposed to returnFalse. 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 asNone != element. That might matter in an exotic case with objects that override__eq__to report being equal toNone. That is highly unlikely. What you most likely looking for is an equivalent toelement is not None. As @pochmann suggested,operator.is_notwould be the best choice.pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
2 remaining items
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
gh-112125: Fix None.__ne__(None) returning NotImplemented instead of …
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
vstinner commentedon Dec 7, 2023
I merged your PR, thank you @andrewluotechnologies. I backported the change manually to 3.12 to fix a merge conflict.
[3.12] gh-112125: Fix None.__ne__(None) returning NotImplemented inst…
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
[explorer/nodewatch]: replace None.__ne__ with lambda expression