Handle NotImplemented correctly in ErrorDetail.__ne__

[allanlewis]

As per the sole commit:

PR #7531 resolved issue #7433 by updating ErrorDetails.__eq__ to correctly handle the NotImplemented case. However, Python 3.9 continues to issue the following warning:

DeprecationWarning: NotImplemented should not be used in a boolean context

This is because __ne__ still doesn't handle the NotImplemented case correctly. In order to avoid this warning, this commit makes the same change for __ne__ as previously made for __eq__.

refs #7433 #7531

[tomchristie]

Wait. Haven't we got both of these wrong? In what context is NotImplemented returned?
Surely it should be raised as an exception.

What code is returning it?

[tomchristie]

Test - does this unapprove?

[allanlewis]

Test - does this unapprove?

You need to "dismiss review" from the panel at the bottom.

Wait. Haven't we got both of these wrong? In what context is NotImplemented returned?
Surely it should be raised as an exception.

What code is returning it?

No, we've got this right, see the docs:

NotImplemented
A special value which should be returned by the binary special methods (e.g. __eq__(), __lt__(), __add__(), __rsub__(), etc.) to indicate that the operation is not implemented with respect to the other type; may be returned by the in-place binary special methods (e.g. __imul__(), __iand__(), etc.) for the same purpose. It should not be evaluated in a boolean context. NotImplemented is the sole instance of the types.NotImplementedType type.
...
Changed in version 3.9: Evaluating NotImplemented in a boolean context is deprecated. While it currently evaluates as true, it will emit a DeprecationWarning. It will raise a TypeError in a future version of Python.

[tomchristie]

Gotcha, thanks.

[allanlewis]

As I said in #7433 (comment) I'm happy to add a test for this case, given some guidance.

[tomchristie]

Perhaps we're okay with it not having one.
PR #7531 didn't.
Consistency is a virtue, no?

[wieczorek1990]

Please rename „r” to „result” or something else.

[tomchristie]

@wieczorek1990 The change here is consistent with the existing code above.

[allanlewis]

Thanks, @tomchristie 🙏

[wieczorek1990]

I don’t think short names are good code.

[tomchristie]

Generally agreed.
A pull request changing them for both cases would be accepted.

[wieczorek1990]

@tomchristie Here: #8585