gh-137508: Unify error messages for index() and remove() #139696
Conversation
Unify error messages for the index() and remove() methods of classes list, tuple, range, memoryview, str, bytes, bytearray, array.array, and collections.deque, and the operator.indexOf() function. Improve error message for xml.etree.ElementTree.Element.remove().
| return NULL; | ||
| } | ||
| PyErr_SetString(PyExc_ValueError, "list.remove(x): x not in list"); | ||
| PyErr_SetString(PyExc_ValueError, "value not in list"); |
There was a problem hiding this comment.
To me this just makes the messages slightly less informative. Personally, I don't see this as an improvement.
There was a problem hiding this comment.
I am not so strong proponent of this change, but I think that there is a benefit of having unified error messages whether it is possible. I'll left this PR until we have an overwhelming support of any variant.
| } | ||
| } | ||
| PyErr_SetString(PyExc_ValueError, "deque.index(x): x not in deque"); | ||
| PyErr_SetString(PyExc_ValueError, "value not in deque"); |
There was a problem hiding this comment.
I prefer the message be left as-is.
There was a problem hiding this comment.
I would indeed prefer having a unified message, but I honestly don't see whether <class>.<meth>(value): value not in <class> is better or not than value not in <class>.
- If I write
obj.index(t)(as is), and 't' is not found, then the traceback would includeobj.indexso indicatinglist.index(value): value not in listwould be redundant. - However, if I were to write
f = obj.index; f(t), then havinglist.indexas a prelude would be much more helpful for the traceback (especially if it's used as a callback for instance).
I don't see many cases where .index() would be used as a callback and where we wouldn't have a hardcoded .index() call, but I can see where we could have .remove() could be used as a callback. The question is whether this is common or not.
Now, I should note that in Python 3.13 we didn't include list.index(x) when reporting an error but we did so in Python 3.14. As such, I think we should continue doing the same. I have a slight preference for x instead of value even if x is usually used for numbers because it reduces the number of characters to read and makes the message shorter in case of a small screen (or a long class name).
To alleviate the issue on sequence.index(x): x not in sequence, I would suggest using "%T.index(x): x not in %T" as the format string. We may or may not decide to include the module name as well so it's still debatable.
However, one thing I would definitely support, is a unified error message, whether we include the prefix <class>.<meth>(...) or not.
3 participants
Unify error messages for the index() and remove() methods of classes list, tuple, range, memoryview, str, bytes, bytearray, array.array, and collections.deque, and the operator.indexOf() function.
Improve error message for xml.etree.ElementTree.Element.remove().
<sequence>.<method>()failures #137508