-
-
Notifications
You must be signed in to change notification settings - Fork 34k
bpo-13349: Fix error reporting for index and remove methods #876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@DimitrisJim, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rhettinger, @serhiy-storchaka and @eliben to be potential reviewers. |
|
Interesting issue! |
|
@SylvainDe About |
|
@DimitrisJim Sure ! Thanks for the answer :-) |
|
I'm not sure this is a good idea. Do we have any evidence that the current error messages are causing problems for users. At least some of this code is well over a decade old and I don't recall a single user complaint. In addition, I don't think we necessarily want to make %.100R the new norm except in cases where the original author thought is would be helpful. |
|
Hi @rhettinger! Personally, I haven't seen any users facing problems due to the specific error message. This is probably because it is generally easy to use On the other hand, it's a shame to have the information on what value was attempted to be removed/looked-up with As for |
Fixes error reporting for the
indexandremovemethods inlist,tuple,deque,operator.indexOf,elementand inabstract.c. Also fixes an occurence of this message in thedoctestdocumentation.Instead of repeating the method name as in the original patch supplied by Sean.Ochoa on b.p.o, it changes all occurences of:
to the more concice:
the repr is trimmed to 100 characters as suggested on b.p.o.
Added necessary tests to check for the presence of the value in the exception.
Specifically, for
.index:test_indexinseq_testsruns for:list.index,tuple.indexanddeque.indextest_indexintest_arrayfor:array.indextest_indexOfintest_operatorfor:operator.indexOfWhile, for
.remove:test_removeinlist_testsfor:list.removetest_removeintest_dequefor:deque.removetest_removeintest_arrayfor:array.removetest_simpleopsintest_xml_etreefor:element.removeThe only open question I currently have is: