classification
Title: Better warnings exception for bad category
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: berker.peksag Nosy List: berker.peksag, brett.cannon, ezio.melotti, pelson, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2012-11-01 14:16 by pelson, last changed 2014-07-11 16:55 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
pelson_warnings_fix.diff pelson, 2012-11-01 14:16 Lib/warnings.py, Python/_warnings.c & Lib/test/test_warnings.py changes (vn 1) review
pelson_warnings_fix_2.diff pelson, 2012-11-20 16:57 Version 2 of the patch review
pelson_warnings_fix_3.diff pelson, 2012-11-22 10:43 Version 3 of the patch (Minor review actions from Berker Peksag) review
pelson_warnings_fix_4.diff pelson, 2013-03-04 13:09 review
issue16382_v5.diff berker.peksag, 2014-06-24 18:31 review
Messages (5)
msg174421 - (view) Author: Phil Elson (pelson) * Date: 2012-11-01 14:16
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!
msg183443 - (view) Author: Phil Elson (pelson) * Date: 2013-03-04 13:09
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,
msg221478 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-06-24 18:31
Here's a new patch addressing Ezio's Rietveld comment. I've also used assertRaisesRegex instead of assertRaises in tests.
msg222762 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-07-11 16:51
Thanks for the patch, Phil.
msg222763 - (view) Author: Roundup Robot (python-dev) Date: 2014-07-11 16:55
New changeset c4a86fe52006 by Berker Peksag in branch 'default':
Issue #16382: Improve exception message of warnings.warn() for bad category.
http://hg.python.org/cpython/rev/c4a86fe52006
History
Date User Action Args
2014-07-11 16:55:16python-devsetnosy: + python-dev
messages: + msg222763
2014-07-11 16:51:59berker.peksagsetstatus: open -> closed

type: behavior -> enhancement
assignee: berker.peksag

nosy: + r.david.murray
messages: + msg222762
resolution: fixed
stage: patch review -> resolved
2014-06-24 18:31:18berker.peksagsetfiles: + issue16382_v5.diff
nosy: + berker.peksag
messages: + msg221478

2014-01-23 21:00:03berker.peksagsetversions: + Python 3.5, - Python 3.4
2013-03-04 13:09:13pelsonsetfiles: + pelson_warnings_fix_4.diff

messages: + msg183443
2012-11-22 10:43:08pelsonsetfiles: + pelson_warnings_fix_3.diff
2012-11-20 16:57:23pelsonsetfiles: + pelson_warnings_fix_2.diff
2012-11-02 19:03:40ezio.melottisetnosy: + ezio.melotti

stage: patch review
2012-11-01 18:05:34pitrousetnosy: + brett.cannon
2012-11-01 14:16:46pelsoncreate