Skip to content
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

format error messages should provide context information #64723

Closed
bitdancer opened this issue Feb 5, 2014 · 7 comments
Closed

format error messages should provide context information #64723

bitdancer opened this issue Feb 5, 2014 · 7 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@bitdancer
Copy link
Member

BPO 20524
Nosy @ericvsmith, @ezio-melotti, @bitdancer, @sobolevn, @iritkatriel
PRs
  • bpo-20524: adds better error message for .format() #28310
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-09-24.15:19:16.167>
    created_at = <Date 2014-02-05.23:23:15.873>
    labels = ['type-bug', 'library', '3.11']
    title = 'format error messages should provide context information'
    updated_at = <Date 2021-09-24.15:19:16.166>
    user = 'https://github.com/bitdancer'

    bugs.python.org fields:

    activity = <Date 2021-09-24.15:19:16.166>
    actor = 'eric.smith'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-09-24.15:19:16.167>
    closer = 'eric.smith'
    components = ['Library (Lib)']
    creation = <Date 2014-02-05.23:23:15.873>
    creator = 'r.david.murray'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 20524
    keywords = ['patch']
    message_count = 7.0
    messages = ['210351', '210354', '210359', '401606', '401700', '402568', '402569']
    nosy_count = 5.0
    nosy_names = ['eric.smith', 'ezio.melotti', 'r.david.murray', 'sobolevn', 'iritkatriel']
    pr_nums = ['28310']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue20524'
    versions = ['Python 3.11']

    @bitdancer
    Copy link
    Member Author

    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()

    @bitdancer bitdancer added the stdlib Python modules in the Lib dir label Feb 5, 2014
    @ezio-melotti ezio-melotti added the type-bug An unexpected behavior, bug, or error label Feb 5, 2014
    @ericvsmith
    Copy link
    Member

    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.

    @ericvsmith
    Copy link
    Member

    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.

    @iritkatriel
    Copy link
    Member

    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 "<stdin>", line 1, in <module>
    ValueError: Invalid format specifier

    @iritkatriel iritkatriel added the 3.11 only security fixes label Sep 10, 2021
    @sobolevn
    Copy link
    Member

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

    I've decided to take an easy path: #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 :)

    @ericvsmith
    Copy link
    Member

    New changeset 8d87291 by Nikita Sobolev in branch 'main':
    bpo-20524: adds better error message for .format() (GH-28310)
    8d87291

    @ericvsmith
    Copy link
    Member

    Thanks for the improvement, @sobolevn!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants