This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Special-case NoneType() in do_richcompare()
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Shannon, ezio.melotti, martin.panter, python-dev, r.david.murray, serhiy.storchaka, vstinner
Priority: low Keywords: easy, patch

Created on 2015-09-22 05:48 by ezio.melotti, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
richcompare.patch vstinner, 2015-10-12 22:33 review
Messages (25)
msg251288 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2015-09-22 05:48
do_richcompare() has a comment (at Objects/object.c:689) that reads:

/* XXX Special-case None so it doesn't show as NoneType() */

This refers to the following error message:

>>> 3 < None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unorderable types: int() < NoneType()

This is a common error that comes up while ordering/sorting, and NoneType() is potentially confusing, so I find the request reasonable.
If the consensus is favourable, it should be implemented, if not, the comment should be removed.
msg251290 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-22 06:16
I find the request reasonable too.
msg251298 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2015-09-22 09:26
+1 for special casing None.
msg251302 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-22 10:16
> TypeError: unorderable types: int() < NoneType()

The error message looks good to me: NoneType is the type of the None singleton:

>>> type(None)
<class 'NoneType'>
>>> x=type(None)()
>>> x is None
True

Do you propose to use the message "TypeError: unorderable types: int() < None"? This message looks wrong to me, None is not a type.

The strange thing is the trailing parenthesis!? Why not "int < NoneType"?
msg251440 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-23 19:23
There are other places NoneType shows up (eg: attribute error).  Why should this case be special?  Do we want to fix them all?
msg251446 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-23 20:23
R. David Murray wrote: "There are other places NoneType shows up (eg: attribute error).  Why should this case be special?  Do we want to fix them all?"

I think you missed the last sentence of the initial message :-) We are simply discussing a comment :-D

Ezio wrote: "if not, the comment should be removed."

Come on, just remove the comment, that's all ;-)
msg251456 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-23 21:12
I was agreeing with you, I think the comment should just be dropped.  My point was the alternative is really to change it everywhere, not just here :)
msg251487 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2015-09-24 06:07
This case is different from most of the others though, because while it talks about unorderable types, it provides an example showing two instances (hence the parentheses).

In these NoneType is correct:
>>> int(None)
TypeError: int() argument must be a string or a number, not 'NoneType'
>>> abs(None)
TypeError: bad operand type for abs(): 'NoneType'
>>> [][None]
TypeError: list indices must be integers, not NoneType

In this NoneType() is equivalent to the None singleton (and int() is a not-better-specified instance of int):
>>> 3 < None
TypeError: unorderable types: int() < NoneType()

So I would either special-case None, remove the () and the comment, or show both the types and the repr() of the two objects.
msg251488 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-24 07:00
And this is the only case where type name followed with '()' is occurred in error message. The repr() shouldn't be used in error message because it can be too long (imagine 'x'*10**9 < None) and can raise an exception.

IMHO more correct would be message "unorderable types: int and NoneType", but it lose the information about operation.

Yet one option is to follow the template for other binary operators:

>>> 1 + None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'
msg251498 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-24 07:52
+1 to remove parenthesis from type names in the error message. But
from all types, not only NoneType.
msg251524 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-24 13:57
See issue 20077.  IMO it should still say int < NoneType, to be consistent.  I still don't see a reason to special case None here.
msg251528 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-24 14:24
int < NoneType is not the same as int() < None.
msg251531 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-24 14:28
Stupid email interface, it restored the the previous title. I tried to change the title to: "Fix do_richcompare() error message: remove parenthesis from type names"
msg251532 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-24 14:33
Serhiy: it's not the same, but the both are correct and parallel, and int < NoneType is *consistent* with the rest of the message, whereas having the parenthesis in there is not (since in that case you are referring to instances, not types).

Changing the message format to be more like the other binary op messages might be better, though.
msg251533 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-24 15:15
It makes the error message just non-relevant (or misleading) to the error.
It refers to int < NoneType, but actually the exception is raised when you 
compare an int instance with a NoneType instance, not two types theirself. 
Comparing *any two types* raises an exception, but comparing instances of two 
types can return a result.
msg251536 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-24 15:45
That's why I said that making it like the other binary op messages might be better.  Currently the error message says that the error is that two types were compared ("unorderable types"), which as you point is not quite accurate.  It is, however, fairly parallel to the message used for binary operations:

   TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'

I think it would be good to change the existing message to match:

   TypeError: unsupported operand type(s) for '<': 'int' and 'NoneType'

The *best* change would be something like:

   TypeError: '<' does not support comparison of operands whose types are 'int' and 'NoneType'
   TypeError: '+' does not support addition of operands whose types are 'int' and 'NoneType'

But that might be much harder to implement depending on how the binary op error message(s) are implemented.  Leave out "addition of" and "comparison of" in the above messages and they might still be better than the status quo, but probably not enough to warrant doing it if doing it is hard.
msg251569 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2015-09-25 03:41
TypeError: '<' not supported between instances of 'int' and 'NoneType'
msg251585 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-09-25 15:26
As I said to Ezio on irc, I like his formulation.
msg252898 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-12 22:33
> TypeError: '<' not supported between instances of 'int' and 'NoneType'

Here is a patch for Python 3.6.
msg252902 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-13 01:46
I like the patch.

It looks like some of the example sessions in the documentation will become out of date. Not sure if that is a problem.
msg252904 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2015-10-13 02:46
LGTM.
Updating examples is low-priority but I think it should be done.
I think it would be good to also add a couple of tests to make sure that the correct operator and types are included in the message.
Both make good bite-sized tasks for new contributors.
msg253006 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-14 16:29
New changeset 0238eafb68da by Victor Stinner in branch 'default':
Issue #25210: Change error message of do_richcompare()
https://hg.python.org/cpython/rev/0238eafb68da
msg253007 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-14 16:29
> It looks like some of the example sessions in the documentation will become out of date. Not sure if that is a problem.

Ah yes, I updated them. Thanks for the review.
msg253041 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2015-10-15 11:00
Thanks!
msg253315 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-22 04:49
New changeset 5c7dd88cd0c5 by Berker Peksag in branch 'default':
Issue #25210: Add some basic tests for the new exception message
https://hg.python.org/cpython/rev/5c7dd88cd0c5
History
Date User Action Args
2022-04-11 14:58:21adminsetgithub: 69397
2015-10-22 04:49:18python-devsetmessages: + msg253315
2015-10-15 11:00:37ezio.melottisetmessages: + msg253041
stage: patch review -> resolved
2015-10-14 16:29:26vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg253007
2015-10-14 16:29:07python-devsetnosy: + python-dev
messages: + msg253006
2015-10-13 02:46:16ezio.melottisetmessages: + msg252904
2015-10-13 01:46:36martin.pantersetnosy: + martin.panter

messages: + msg252902
stage: needs patch -> patch review
2015-10-12 22:33:57vstinnersetfiles: + richcompare.patch
keywords: + patch
messages: + msg252898
2015-09-25 15:26:53r.david.murraysetmessages: + msg251585
2015-09-25 03:41:56ezio.melottisetmessages: + msg251569
2015-09-24 15:45:31r.david.murraysetmessages: + msg251536
2015-09-24 15:15:59serhiy.storchakasetmessages: + msg251533
2015-09-24 14:33:44r.david.murraysetmessages: + msg251532
2015-09-24 14:28:56vstinnersetmessages: + msg251531
2015-09-24 14:24:54serhiy.storchakasetmessages: + msg251528
title: Fix do_richcompare() error message: remove parenthesis from type names -> Special-case NoneType() in do_richcompare()
2015-09-24 14:04:01vstinnersettitle: Special-case NoneType() in do_richcompare() -> Fix do_richcompare() error message: remove parenthesis from type names
2015-09-24 13:57:46r.david.murraysetmessages: + msg251524
2015-09-24 07:52:14vstinnersetmessages: + msg251498
2015-09-24 07:00:06serhiy.storchakasetmessages: + msg251488
2015-09-24 06:07:04ezio.melottisetmessages: + msg251487
2015-09-23 21:12:20r.david.murraysetmessages: + msg251456
2015-09-23 20:23:43vstinnersetmessages: + msg251446
2015-09-23 19:23:30r.david.murraysetnosy: + r.david.murray
messages: + msg251440
2015-09-22 10:16:48vstinnersetnosy: + vstinner
messages: + msg251302
2015-09-22 09:26:26Mark.Shannonsetnosy: + Mark.Shannon
messages: + msg251298
2015-09-22 06:16:13serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg251290
stage: test needed -> needs patch
2015-09-22 05:48:47ezio.melotticreate