gh-130821: Add type information to wrong type error messages #130835
Conversation
it seems that we don't check the text of error messages if we don't want to check the error type itself |
| @@ -1421,17 +1423,17 @@ _PyNumber_Index(PyObject *item) | |||
|
|
|||
| if (!PyLong_Check(result)) { | |||
| PyErr_Format(PyExc_TypeError, | |||
| "__index__ returned non-int (type %.200s)", | |||
There was a problem hiding this comment.
I don't see much reason to change this error message either.
There was a problem hiding this comment.
I changed it to the more common way as you recommended elsewhere: __method__() must return an int, not ...
and added type info before method name, like for other methods
| @@ -2435,10 +2436,8 @@ method_output_as_list(PyObject *o, PyObject *meth) | |||
| PyThreadState *tstate = _PyThreadState_GET(); | |||
| if (_PyErr_ExceptionMatches(tstate, PyExc_TypeError)) { | |||
| _PyErr_Format(tstate, PyExc_TypeError, | |||
| "%.200s.%U() returned a non-iterable (type %.200s)", | |||
There was a problem hiding this comment.
Here I strongly prefer the old message. Iterable is not a type, it's a category of types.
There was a problem hiding this comment.
what do you think about new message: "%T.%U() must return an iterable, not %T"
There was a problem hiding this comment.
I don't think new message adds something bad. Why someone might think, that iterable is a type here? (Another similar case, where new messages mention an iterator.)
There was a problem hiding this comment.
Why someone might think, that iterable is a type here?
it's just that in the previous iteration the error message was the phrase "must return type ..." :)
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
Thank you everyone for review |
I got next output for use case from issue:
but we still have a lot of other places where wrong type errors have inconsistent messages. should they be updated too?