Repository navigation
assertRaises as a context manager should accept a 'msg' keyword argument. #54984
Description
| BPO | 10775 |
|---|---|
| Nosy | @rhettinger, @ezio-melotti, @bitdancer, @voidspace, @briancurtin, @durban |
| Files |
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/ezio-melotti'
closed_at = <Date 2011-05-06.12:07:25.291>
created_at = <Date 2010-12-26.19:18:47.677>
labels = ['easy', 'type-feature']
title = "assertRaises as a context manager should accept a 'msg' keyword argument."
updated_at = <Date 2011-05-06.12:07:25.290>
user = 'https://github.com/bitdancer'bugs.python.org fields:
activity = <Date 2011-05-06.12:07:25.290>
actor = 'ezio.melotti'
assignee = 'ezio.melotti'
closed = True
closed_date = <Date 2011-05-06.12:07:25.291>
closer = 'ezio.melotti'
components = []
creation = <Date 2010-12-26.19:18:47.677>
creator = 'r.david.murray'
dependencies = []
files = ['20765', '21839']
hgrepos = []
issue_num = 10775
keywords = ['patch', 'easy']
message_count = 15.0
messages = ['124675', '125169', '128623', '130840', '130897', '130907', '130943', '130966', '130968', '130970', '130988', '134849', '135276', '135278', '135279']
nosy_count = 10.0
nosy_names = ['rhettinger', 'ezio.melotti', 'r.david.murray', 'michael.foord', 'brian.curtin', 'SilentGhost', 'daniel.urban', 'Winston.Ewert', 'python-dev', 'robquad']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue10775'
versions = ['Python 3.3']
Activity
bitdancer commentedon Dec 26, 2010
assertRaises used as a method can't take a msg keyword argument because all args and keywords are passed to the callable. But in context manager form it could, and this can be useful. See, for example, bpo-3583.
voidspace commentedon Jan 3, 2011
I'm fine with this functionality being added in 3.3.
WinstonEwert commentedon Feb 16, 2011
I decided to try my hand at writing a patch for python.
I ended up implementing the behavior for assertRaises, assertRaisesRegex, assertWarns, and assertWarnsRegex. I also made those functions complain about other arguments rather then just ignoring them.
robquad commentedon Mar 14, 2011
Changing callableObj to callable_obj in assertRaises will break for anyone that's upgrading to 3.3. I left a comment on the review.
ncoghlan commentedon Mar 14, 2011
Aren't such use cases already covered by assertRaisesRegex?
bitdancer commentedon Mar 14, 2011
How does assertRaisesRegex address the use case in bpo-3583?
WinstonEwert commentedon Mar 15, 2011
robquad mentions having left a comment on the review, but I'm not seeing how to view it. Can somebody explain?
It wasn't necessary to change the callable_obj bit, but both form were being used so I thought it best to standardize. Neither version of the parameter name shows up in the documentation.
ncoghlan commentedon Mar 15, 2011
Michael pointed out that I had completely missed the point of what the "msg" argument was about. Sorry for the noise.
SilentGhost commentedon Mar 15, 2011
briancurtin commentedon Mar 15, 2011
I showed robquad how to do the review stuff at PyCon but I forgot about the publish part. Robbie, if you hit "Publish + Mail Comments" near the top of the page after you've left comments, it'll send them out.
What he noticed was that changing to callable_obj in the assertRaises signature could break anyone who had been using callableObj as a named argument. Although it isn't explicitly documented, it's a named argument that someone is probably using. As for standardizing, it's probably best to match the general format of the library which is camelCase, and change the internal uses rather than a public method signature.
WinstonEwert commentedon Mar 15, 2011
The public methods were using both callable_obj and callableObj. Perhaps the patch should standardize on callableObj and accept callable_obj with a warning?
ezio-melotti commentedon Apr 30, 2011
Attached a revised patch.
While I agree that an error should be raised when extra args are provided in the context manager form, this is out of the scope of the issue, so I didn't include those changes.
voidspace commentedon May 6, 2011
New patch by Ezio looks good to me. Go ahead and commit. Please raise a separate issue for error reporting when invalid argument combinations are used. (i.e. additional keyword arguments but no callable.)
python-dev commentedon May 6, 2011
New changeset 8fc801ca9ea1 by Ezio Melotti in branch 'default':
Issue bpo-10775: assertRaises, assertRaisesRegex, assertWarns, and assertWarnsRegex now accept a keyword argument 'msg' when used as context managers. Initial patch by Winston Ewert.
http://hg.python.org/cpython/rev/8fc801ca9ea1
ezio-melotti commentedon May 6, 2011
Done, thanks for the patch!