msg251288 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
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) * |
Date: 2015-09-22 06:16 |
I find the request reasonable too.
|
msg251298 - (view) |
Author: Mark Shannon (Mark.Shannon) * |
Date: 2015-09-22 09:26 |
+1 for special casing None.
|
msg251302 - (view) |
Author: STINNER Victor (vstinner) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2015-09-24 14:24 |
int < NoneType is not the same as int() < None.
|
msg251531 - (view) |
Author: STINNER Victor (vstinner) * |
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) * |
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) * |
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) * |
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) * |
Date: 2015-09-25 03:41 |
TypeError: '<' not supported between instances of 'int' and 'NoneType'
|
msg251585 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2015-09-25 15:26 |
As I said to Ezio on irc, I like his formulation.
|
msg252898 - (view) |
Author: STINNER Victor (vstinner) * |
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) * |
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) * |
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) |
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) * |
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) * |
Date: 2015-10-15 11:00 |
Thanks!
|
msg253315 - (view) |
Author: Roundup Robot (python-dev) |
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
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:21 | admin | set | github: 69397 |
2015-10-22 04:49:18 | python-dev | set | messages:
+ msg253315 |
2015-10-15 11:00:37 | ezio.melotti | set | messages:
+ msg253041 stage: patch review -> resolved |
2015-10-14 16:29:26 | vstinner | set | status: open -> closed resolution: fixed messages:
+ msg253007
|
2015-10-14 16:29:07 | python-dev | set | nosy:
+ python-dev messages:
+ msg253006
|
2015-10-13 02:46:16 | ezio.melotti | set | messages:
+ msg252904 |
2015-10-13 01:46:36 | martin.panter | set | nosy:
+ martin.panter
messages:
+ msg252902 stage: needs patch -> patch review |
2015-10-12 22:33:57 | vstinner | set | files:
+ richcompare.patch keywords:
+ patch messages:
+ msg252898
|
2015-09-25 15:26:53 | r.david.murray | set | messages:
+ msg251585 |
2015-09-25 03:41:56 | ezio.melotti | set | messages:
+ msg251569 |
2015-09-24 15:45:31 | r.david.murray | set | messages:
+ msg251536 |
2015-09-24 15:15:59 | serhiy.storchaka | set | messages:
+ msg251533 |
2015-09-24 14:33:44 | r.david.murray | set | messages:
+ msg251532 |
2015-09-24 14:28:56 | vstinner | set | messages:
+ msg251531 |
2015-09-24 14:24:54 | serhiy.storchaka | set | messages:
+ msg251528 title: Fix do_richcompare() error message: remove parenthesis from type names -> Special-case NoneType() in do_richcompare() |
2015-09-24 14:04:01 | vstinner | set | title: Special-case NoneType() in do_richcompare() -> Fix do_richcompare() error message: remove parenthesis from type names |
2015-09-24 13:57:46 | r.david.murray | set | messages:
+ msg251524 |
2015-09-24 07:52:14 | vstinner | set | messages:
+ msg251498 |
2015-09-24 07:00:06 | serhiy.storchaka | set | messages:
+ msg251488 |
2015-09-24 06:07:04 | ezio.melotti | set | messages:
+ msg251487 |
2015-09-23 21:12:20 | r.david.murray | set | messages:
+ msg251456 |
2015-09-23 20:23:43 | vstinner | set | messages:
+ msg251446 |
2015-09-23 19:23:30 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg251440
|
2015-09-22 10:16:48 | vstinner | set | nosy:
+ vstinner messages:
+ msg251302
|
2015-09-22 09:26:26 | Mark.Shannon | set | nosy:
+ Mark.Shannon messages:
+ msg251298
|
2015-09-22 06:16:13 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka
messages:
+ msg251290 stage: test needed -> needs patch |
2015-09-22 05:48:47 | ezio.melotti | create | |