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

Better warnings exception for bad category #60586

Closed
pelson mannequin opened this issue Nov 1, 2012 · 5 comments
Closed

Better warnings exception for bad category #60586

pelson mannequin opened this issue Nov 1, 2012 · 5 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@pelson
Copy link
Mannequin

pelson mannequin commented Nov 1, 2012

BPO 16382
Nosy @brettcannon, @ezio-melotti, @bitdancer, @berkerpeksag, @pelson
Files
  • pelson_warnings_fix.diff: Lib/warnings.py, Python/_warnings.c & Lib/test/test_warnings.py changes (vn 1)
  • pelson_warnings_fix_2.diff: Version 2 of the patch
  • pelson_warnings_fix_3.diff: Version 3 of the patch (Minor review actions from Berker Peksag)
  • pelson_warnings_fix_4.diff
  • issue16382_v5.diff
  • 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 = 'https://github.com/berkerpeksag'
    closed_at = <Date 2014-07-11.16:51:59.126>
    created_at = <Date 2012-11-01.14:16:46.324>
    labels = ['type-feature', 'library']
    title = 'Better warnings exception for bad category'
    updated_at = <Date 2014-07-11.16:55:16.905>
    user = 'https://github.com/pelson'

    bugs.python.org fields:

    activity = <Date 2014-07-11.16:55:16.905>
    actor = 'python-dev'
    assignee = 'berker.peksag'
    closed = True
    closed_date = <Date 2014-07-11.16:51:59.126>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2012-11-01.14:16:46.324>
    creator = 'pelson'
    dependencies = []
    files = ['27824', '28053', '28072', '29304', '35772']
    hgrepos = []
    issue_num = 16382
    keywords = ['patch']
    message_count = 5.0
    messages = ['174421', '183443', '221478', '222762', '222763']
    nosy_count = 6.0
    nosy_names = ['brett.cannon', 'ezio.melotti', 'r.david.murray', 'python-dev', 'berker.peksag', 'pelson']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16382'
    versions = ['Python 3.5']

    @pelson
    Copy link
    Mannequin Author

    pelson mannequin commented Nov 1, 2012

    When passing an invalid Warning subclasses to the warnings.warn function, a bare issubclass exception is raised:

    >>> import warnings
    >>> warnings.warn('hello world', 'not a valid warning type')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: issubclass() arg 1 must be a class

    This exception is consistent accross both Python/_warnings.c and Lib/warnings.py implementations, but I feel it could be more helpful/explicit about the nature problem.

    To test both cases I have been using the following code (python3.4):

    >> import test.support
    >> py_warnings = test.warnings = test.support.import_fresh_module('warnings', blocked=['_warnings'])
    >> c_warnings = test.support.import_fresh_module('warnings', fresh=['_warnings'])

    Now:

    >>> py_warnings.warn('hello world', '')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "lib/python3.4/warnings.py", line 168, in warn
        assert issubclass(category, Warning)
    TypeError: issubclass() arg 1 must be a class
    
    >>> c_warnings.warn('hello world', '')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: issubclass() arg 1 must be a class

    Additionally, there is a difference in the denotational semantics of None between the c and py warnings implementation:

    >>> py_warnings.warn('Hello world', None)
    __main__:1: UserWarning: Hello world
    
    >>> c_warnings.warn('Hello world', None)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: issubclass() arg 1 must be a class

    I can understand that python does not allow the concept of an optional positional arguments and therefore it is arguable that the signatures of the two functions are inevitably going to be different. I defer to someone more knowledgeable in Python to decide if this is a problem, and whether it should be made consistent.

    Attached is a patch to address these two issues, with associated tests. Please review (n.b. I am a python developer at heart, and only dabble in C when I have to, so extra scrutiny on the C would be valuable to me) and I'd be happy to get any necessary changed applied to the patch asap.

    In short, as a result of applying this patch, the following results ensue:

    >>> py_warnings.warn('hello world', '')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "lib/python3.4/warnings.py", line 175, in warn
        'Got {!r}'.format(Warning, category)) from None
    ValueError: category must be a subclass of <class 'Warning'>.Got ''
    
    
    >>> c_warnings.warn('hello world', '')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: category must be a subclass of <class 'Warning'>. Got ''.
    >>> 
    >>> c_warnings.warn('hello world', None)
    __main__:1: UserWarning: hello world

    Thanks!

    @pelson pelson mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Nov 1, 2012
    @pelson
    Copy link
    Mannequin Author

    pelson mannequin commented Mar 4, 2013

    Ok. I think I've done all of the actions from the reviews.

    I'm not sure if I should remove the old patches or not?

    Thanks,

    @berkerpeksag
    Copy link
    Member

    Here's a new patch addressing Ezio's Rietveld comment. I've also used assertRaisesRegex instead of assertRaises in tests.

    @berkerpeksag
    Copy link
    Member

    Thanks for the patch, Phil.

    @berkerpeksag berkerpeksag self-assigned this Jul 11, 2014
    @berkerpeksag berkerpeksag added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jul 11, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 11, 2014

    New changeset c4a86fe52006 by Berker Peksag in branch 'default':
    Issue bpo-16382: Improve exception message of warnings.warn() for bad category.
    http://hg.python.org/cpython/rev/c4a86fe52006

    @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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant