[issue20524] format error messages should provide context information

[Eric V. Smith]

Eric V. Smith  added the comment:

Thanks for the improvement, @sobolevn!

--
resolution:  -> fixed
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

[issue20524] format error messages should provide context information

[Eric V. Smith]

Eric V. Smith  added the comment:


New changeset 8d8729146f21f61af66e70d3ae9501ea6bdccd09 by Nikita Sobolev in 
branch 'main':
bpo-20524: adds better error message for `.format()` (GH-28310)
https://github.com/python/cpython/commit/8d8729146f21f61af66e70d3ae9501ea6bdccd09


--

___
Python tracker 

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

[issue20524] format error messages should provide context information

[Nikita Sobolev]

Nikita Sobolev  added the comment:

> I think that would require some major surgery to the code, but would be worth 
> it.

I've decided to take an easy path: https://github.com/python/cpython/pull/28310

```
PyErr_Format(PyExc_ValueError,
 "Invalid format specifier: '%s' for object of type '%s'",
 PyUnicode_AsUTF8AndSize(format_spec, NULL),
 Py_TYPE(obj)->tp_name);
```

This worked for me as the starting point. It covered: int, float, complex, and 
str types.

> Note that in your original example, you want the error to contain 
> "{length:%HH:%MM}". By the time the error is detected, the only thing the 
> code knows is the format specifier "%HH:%MM". It doesn't know the "length" 
> part. 

I guess it has changed since the 2014.

I would love to hear any feedback on my proposal and improve it to the level 
when it can be merged :)

--

___
Python tracker 

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

[issue20524] format error messages should provide context information

[Nikita Sobolev]

Change by Nikita Sobolev :


--
keywords: +patch
nosy: +sobolevn
nosy_count: 4.0 -> 5.0
pull_requests: +26723
stage: test needed -> patch review
pull_request: https://github.com/python/cpython/pull/28310

___
Python tracker 

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

[issue20524] format error messages should provide context information

[Irit Katriel]

Irit Katriel  added the comment:

Reproduced on 3.11:

>>> dd = {'length': 12, 'id': 4, 'title': "t", 'run_time': datetime.time()}
>>> '{run_time:%H:%M:%S}, 
>>> ,COM,DA{id},"{title:.43}",{id},{length:%M:%S}'.format(**dd)
Traceback (most recent call last):
  File "", line 1, in 
ValueError: Invalid format specifier

--
nosy: +iritkatriel
versions: +Python 3.11

___
Python tracker 

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

[issue20524] format error messages should provide context information

[R. David Murray]

New submission from R. David Murray:

Consider the following:

   '{run_time:%H:%M:%S}, 
,COM,DA{id},{title:.43},{id},{length:%M:%S}'.format(**mydict)

The error message I got was:

   Invalid format specifier

The problem turned out to be that the value of the 'length' key was an integer 
instead of a datetime.time(), but it sure wasn't easy to figure out which bit 
of the format string or which variable was the problem.

It would be nice for the format error message to include the pattern that it is 
parsing when it hits the error.  The type of the value being substituted would 
also be nice.  Perhaps something like:

   The format specifier in {length:%HH:%MM} is not valid for type int()

--
components: Library (Lib)
messages: 210351
nosy: eric.smith, r.david.murray
priority: normal
severity: normal
status: open
title: format error messages should provide context information

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

[issue20524] format error messages should provide context information

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
nosy: +ezio.melotti
stage:  - test needed
type:  - behavior

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

[issue20524] format error messages should provide context information

[Eric V. Smith]

Eric V. Smith added the comment:

That would be a great improvement. It's in Python/formatter_unicode.c, line 
245, in parse_internal_render_format_spec().

That code knows about the format spec, but not the type being formatted. That 
would be easy enough to pass in.

This fix would only work for the built-in types: int, float, and str, I think. 
Maybe complex. But that's probably good enough.

--

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

[issue20524] format error messages should provide context information

[Eric V. Smith]

Eric V. Smith added the comment:

int, float, str, and complex are the types formatted by that code.

Notice that Decimal already has a better message:

 format(Decimal(42), 'tx')
Traceback (most recent call last):
  ...
ValueError: Invalid format specifier: tx

 format(42, 'tx')
Traceback (most recent call last):
  ...
ValueError: Invalid conversion specification

But, look at this:
 format(3, '--')
Traceback (most recent call last):
  File stdin, line 1, in module
ValueError: Unknown format code '-' for object of type 'int'

This is generated in unknown_presentation_type, also in formatter_unicode.c. It 
almost does what you want, but just handles the presentation type, not the 
whole format specifier.

Error handling could be cleaned up in that module. I'd say that the string 
should be:
specific error with format specifier specifier for object of type 'type'

specific error might be Unknown presentation type '-', or Cannot specify 
','.

I think that would require some major surgery to the code, but would be worth 
it.

Note that in your original example, you want the error to contain 
{length:%HH:%MM}. By the time the error is detected, the only thing the code 
knows is the format specifier %HH:%MM. It doesn't know the length part. The 
error is basically in int.__format__. By the time that gets called, the format 
specifier has already been extracted and the argument selection (by indexing, 
by name, including attribute access) has already taken place.

--

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