[issue13349] Non-informative error message in index() and remove() functions

[Jim Fasarakis-Hilliard]

Jim Fasarakis-Hilliard added the comment:

Thanks, I understand why this isn't the best idea now.

> shouldn't we change error messages that contains the repr of not found value? 

That is what I was thinking too, apart from removing the repr from other 
messages it will also unify reporting for all these methods.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I concur with Raymond.

But shouldn't we change error messages that contains the repr of not found 
value? Affected collections are list, deque and range.

>>> [].index('spam')
Traceback (most recent call last):
  File "", line 1, in 
ValueError: 'spam' is not in list
>>> import collections
>>> collections.deque().index('spam')
Traceback (most recent call last):
  File "", line 1, in 
ValueError: 'spam' is not in deque

range.index() raises different error messages depending on the type of the 
value:

>>> range(10).index('spam')
Traceback (most recent call last):
  File "", line 1, in 
ValueError: sequence.index(x): x not in sequence
>>> range(10).index(42)
Traceback (most recent call last):
  File "", line 1, in 
ValueError: 42 is not in range

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Nick Coghlan]

Nick Coghlan added the comment:

While I agree it's reasonable to keep arbitrary value repr's out of these 
messages by default, I do think it would be desirable for someone sufficiently 
motivated to file a separate RFE about adding the value as a structured 
exception attribute so that custom sys.excepthook implementations and other 
"unhandled runtime exception" loggers may choose to handle the situation 
differently.

This wouldn't be about the cases where code is handling the remove() or index() 
call directly, but rather about those where third party library code isn't 
handling missing values correctly, so the debugging scenario a developer is 
faced with looks more like this than it does the examples in the OP here:

```
>>> some_library_function()
Traceback (most recent call last):
  File "", line 1, in 
  File "", line 2, in some_library_function
  File "", line 2, in some_other_library_function
ValueError: list.remove(x): x not in list
```

Getting that kind of traceback in the logs for a production failure in a 
distributed system is all kinds of frustrating since you *know* the interpreter 
knew what "x" was when the error happened, it just isn't passing that 
information along to the developer that's trying to figure out how and why it 
broke.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Raymond Hettinger]

Raymond Hettinger added the comment:

> I'm fine with not changing this

Thanks Brett.  I'm going to mark this as closed/rejected.  After five years, it 
is time to put it to rest and let it stop consuming our mental clock cycles.

> I don't think you're guaranteed to have access to the object
> that triggered the ValueError.

I'm not sure I follow what you're saying.  To call index() or remove() you 
already have to have the object to search for.   The ValueError is raised when 
the searched for object is not found, not by some unknown object inside the 
container.

--
resolution:  -> rejected
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Brett Cannon]

Brett Cannon added the comment:

I'm fine with not changing this, but I did want to say that I don't think 
you're guaranteed to have access to the object that triggered the ValueError. 
If the value that is searched for is in some extension module where it's 
calculated using C code then you have no way of accessing that value unless the 
extension module was nice enough to provide the object in the exception, give 
the repr in the exception message, or document how exactly the transformation 
from input to what you searched for is.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Joe Jevnik]

Changes by Joe Jevnik :


--
pull_requests: +1919

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Raymond Hettinger]

Raymond Hettinger added the comment:

Thinking back over my 17 years of using Python, I really don't think this is 
needed at all.  I can't think of a single situation where it would have helped.

In some cases, the change would be detrimental to readability, introducing 
clutter into an otherwise clear error message.  The OP's example of 
"ValueError: 4 is not in list" is atypical. A more typical example would be 
"ValueError: <__main__.TensorElementPlaceholder object at 0x106489e48> is not 
in list".

As a Python instructor, I've become acutely aware that tracebacks already have 
usability issues due to visual clutter and an unpleasant overall appearance.  
IMO, this proposal would sometimes make it worse by adding more noise for the 
reader to filter out.

Brett's idea is an improvement to the original proposal. By attaching the 
object to the exception rather than squeezing its representation into the 
string, you avoid making the error message hard to read and avoid the 
speed/space costs from computing the __repr__ on arbitrary objects.  That said, 
I don't think there is any programmatic benefit to having the object in the 
exception.  When you call "a.remove(b)", you already have "b" and it would be 
pointless to re-extract it with "except ValueError as e:  b = e.args[-1]".

I'm also concerned that the proposed changes sweep across the core affecting 
code that has been deployed and stable for almost two decades.

Given the dubious benefits and potential detriments, I vote for declining this 
proposal and favoring the stable deployed code that has worked fine for many 
years.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Brett Cannon]

Brett Cannon added the comment:

A potential compromise would be if ValueError gained a 'value' attribute. That 
would allow for the exception message to be static but still provide the 
information for introspection later if desired to figure out exactly what 
object led to the cause of the ValueError. If I remember correctly previous 
objections to this idea was worry about memory, though, so it seems attaching 
more information about the trigger of a ValueError will inevitably lead to more 
cost somehow.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Brett Cannon]

Changes by Brett Cannon :


--
nosy: +brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Raymond Hettinger]

Raymond Hettinger added the comment:

See http://bugs.python.org/issue30477 for why I think this shouldn't be done.  
In short, the IndexError may be part of control flow (looping over successive 
matches) rather than an error (this patch adds a time/space cost with no 
benefit in those cases).  The repr itself may be large resulting in an unusable 
error message.  The repr may be expensive to compute for some objects.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Zahari Dim]

Changes by Zahari Dim :


--
pull_requests: +1418

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Error messages should use repr(). For example str() of bytes object emits a 
warning or error.

See issue26090 for the issue with truncated reprs.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Jim Fasarakis-Hilliard]

Jim Fasarakis-Hilliard added the comment:

Could use `%.100S` instead of `%.100R` but I'm not sure of the downsides that 
might entail.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Jim Fasarakis-Hilliard]

Jim Fasarakis-Hilliard added the comment:

Yes, that does look like too much. My rationale for adding quotes around the 
value was in order to make it more clear in cases where the repr exceeds 100 
characters. 

Instead of:

   Traceback (most recent call last):
 File "", line 1, in 
   ValueError: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 
18, 19, 20, 21, 22, 23, 24, 25, 26, 2 not in list

Have it clearly distinguished by using "":

   Traceback (most recent call last):
 File "", line 1, in 
   ValueError: "[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 
18, 19, 20, 21, 22, 23, 24, 25, 26, 2" not in list

I'm not sure if there's a trivial way to not display so many quotes in the case 
you supplied, you're better suited to decide if this can be done somehow.

--
versions: +Python 3.7 -Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

>>> [1, 2, 3].index("'")
Traceback (most recent call last):
  File "", line 1, in 
ValueError: ""'"" not in list

Aren't too much quotes?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Jim Fasarakis-Hilliard]

Changes by Jim Fasarakis-Hilliard :


--
pull_requests: +775

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Jim Fasarakis-Hilliard]

Jim Fasarakis-Hilliard added the comment:

Additional instances of this:

 - function indexOf of operator.py.
 - function _PySequence_IterSearch of abstract.c

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Emmanuel Arias]

Emmanuel Arias added the comment:

I agree with Jim Fasarakis-Hilliard this message may be change in new 3.7 
version

--
nosy: +eamanu

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Jim Fasarakis-Hilliard]

Jim Fasarakis-Hilliard added the comment:

@Sean Ochoa, do you want to make this into a PR? The only tweak I would suggest 
would be to change all error messages to either be:

"object.method(repr(x)): element not in object" 

or:

"repr(x) not in object"

also, this probably needs to be changed to version 3.7 now.

--
nosy: +Jim Fasarakis-Hilliard

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[STINNER Victor]

STINNER Victor added the comment:

 Confirmed with Ezio that issue #7330 will need to be fixed/approved before 
 this issue can be closed.

FYI I just fixed this issue. PyUnicode_FromFormat() now supports width and 
precision in the format string.

--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Update based on Taggnostr's (Ezio on IRC, if I recall correctly) feedback from 
11/12/2012 around 11:57 PST.  

* Added check for index result in positive tests.
* Added assertIn check for remove result in positive tests.
* Removed extra whitespace.
* Formatted comments to be more concise.

--
Added file: http://bugs.python.org/file28265/issue13349.patch.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

* Fixed issue with test name in Lib/test/test_tuple.py
* Fixed issue with test_remove in Lib/test/test_list.py to assertNotIn instead 
of assertIn for positive case.

Confirmed with Ezio that issue #7330 will need to be fixed/approved before this 
issue can be closed.

--
Added file: http://bugs.python.org/file28267/issue13349.patch.7

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

See also my comments to previous patch about repr() and error message checking.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Lib/test/test_array.py
  -- Moved index test (for this issue) to test_index (existing test method).
  -- Added remove test (for this issue) to test_remove (existing test method)

Lib/test/test_deque.py
  -- Moved remove test (for this issue) to test_index (existing test method).

Lib/test/test_list.py
  -- Added test_remove instead of test_issue13349, and test_index as a new test 
method; both with positive tests and a negative test to for this issue.

Lib/test/test_tuple.py
  -- Added test_remove instead of test_issue13349 as a new test method with a 
positive test and a negative test case for this issue to cover both items 
longer than 100chars and shorter items.

Lib/test/test_xml_etree.py
  -- Added test_remove as a new test method to TreeBasicOperationTest testcase 
class instead of test_issue13349, with a positive test case and the negative 
test to cover this issue.

--
Added file: http://bugs.python.org/file27975/issue13349.patch.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
dependencies: +PyUnicode_FromFormat: implement width and precision for %s, %S, 
%R, %V, %U, %A

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Updates after feedback from Serhiy.

--
Added file: http://bugs.python.org/file27949/issue13349.patch.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Julian Berman]

Julian Berman added the comment:

test_issuewhatever is a bit indescriptive.

It'd be easier for someone glancing at the test to see what it claims to be 
testing if there were a more descriptive name given, like perhaps 
test_exception_message (or something even more verbose).

--
nosy: +Julian

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Or test_index()/test_remove().  In this case positive test cases (and may be 
other exception types) should be added.

--
nosy: +serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Truncating repr strings to 100chars will require the patch from #7330.  After 
applying the patch from that issue, my tests work fine.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Updated patch after taking into account Ezio's (aka Taggnostr on #python-dev) 
latest feedback.

--
Added file: http://bugs.python.org/file27940/issue13349.patch.2

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

From Taggnostr on #python-dev:  

1.) Use assertRaises+assertIn instead of assertRaisesRegex
2.) Add or change an existing test that you've already updated to use  elements 
with a repr longer than 100 chars.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Tests updated for coverage and to use assertRaisesRegex.

--
Added file: http://bugs.python.org/file27907/issue13349.patch.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

After discussing with folks on the #python-dev tonight, I learned that I was 
testing with a list and I should've been using a set.  I'm working on a patch 
now, and I'm almost ready to have it reviewed.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Ready for review.

--
keywords: +patch
Added file: http://bugs.python.org/file27884/issue-13349.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Ezio Melotti]

Ezio Melotti added the comment:

You should use assertRaises/assertRaisesRegex to check the error message and 
add more tests to cover all the changes.  I also noticed an inconsistence 
between 'element not in container' and 'element is not in container' (note the 
extra 'is').  FWIW I prefer the version without 'is'.

--
stage: needs patch - patch review

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Éric Araujo]

Changes by Éric Araujo mer...@netwok.org:


--
title: Uninformal error message in index() and remove() functions - 
Non-informative error message in index() and remove() functions

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

Working on issue as part of Python Bug Day, Oct 2012.

--
nosy: +Sean.Ochoa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Sean Ochoa]

Sean Ochoa added the comment:

It seems that this has been fixed.  Using simple tests from msg147215:  

~/hg/cpython/working $ env-py3.3/bin/python
Python 3.3.0 (default, Nov  3 2012, 15:28:29) 
[GCC 4.7.2] on linux
Type help, copyright, credits or license for more information.
 [].index(list(range(1000)))
Traceback (most recent call last):
  File stdin, line 1, in module
ValueError: [0, 1, 2, 3, 4, 5, ... 995, 996, 997, 998, 999] is not in list
[60649 refs]
 class Foo: pass
... 
[60679 refs]
 [].index(Foo())
Traceback (most recent call last):
  File stdin, line 1, in module
ValueError: __main__.Foo object at 0x7f7fad31bc30 is not in list
[60677 refs]

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13349] Non-informative error message in index() and remove() functions

[Terry J. Reedy]

Terry J. Reedy added the comment:

The original example was about tuples and displaying 'x' instead of '4'. Have 
that and *all* the others been fixed?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13349
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com