-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
IGNORE_EXCEPTION_DETAIL should ignore the module name #51739
Comments
In Python 3.x [1] the exception formatting prints the module path, while Since IGNORE_EXCEPTION_DETAIL was implemented to hide differences I'll attach diffs both for trunk and for py3k-branch, so that both forms [1] And possibly in some cases under Python 2.7 according to reports in [2] http://mail.python.org/pipermail/python-dev/2009-December/094460.html |
The only design level question I can see is as follows: ExceptionName matches ExceptionName (always) Should that 4th case still match under IGNORE_EXCEPTION_DETAIL? My The main reason I think it should match is that it would allow |
My impression is that IGNORE_EXCEPTION_DETAIL is designed to allow you The one argument against it that I can see is the hypothetical case of |
Agreed - particularly since that corner case can still be tested through The patches mostly look good, but the doc changes should be updated to E.g. """Note that :const:`ELLIPSIS` can also be used to ignore the |
Yes, x.y.Exception and a.b.Exception should match. I just realized I |
@lennart: yes, I do think you should add a test for that case. I |
New diff for trunk, with the additional test |
New diff for Py3k with the additional test |
Having this in 2.6/2.7 would be great. I don't think the ELLIPSIS workaround suggested by Barry works, have you actually tried it? Below is an example where ELLIPSIS doesn't seem to help (run in 2.6.5). I have also tried "...Error:" and "...:", and tried replacing ". . ." by "...", to no avail. I'm assuming this has to do with issue bpo-1192554, or am I making a silly mistake? Otherwise, are there any other workarounds you can suggest? Without ellipsis the following example works in 2.6 but of course fails in 3.x. Failed example:
Redacted.from_str('1-7@') #doctest: +ELLIPSIS
Expected:
Traceback (most recent call last):
. . .
...ParserError: <message redacted>:
1-7@
^
expected digit
Got:
Traceback (most recent call last):
<SNIP>
ParserError: <message redacted>:
1-7@
^
expected digit |
The ellipsis doesn't work, because when you have an ellipsis at the beginning of the message, doctest will not understand that it's supposed to be an Exception, so it doesn't even try to match exceptions, and it will therefore always fail. |
Here's a better example that you can cut and paste. import optparse
def foo():
"""
>>> foo() #doctest: +ELLIPSIS
Traceback (most recent call last):
. . .
...OptionError: option bar: foo
"""
raise optparse.OptionError('foo', 'bar')
if __name__ == "__main__":
import doctest
doctest.testmod() |
Ah, right... so there is no easy workaround at present? |
Sure: Catch the exception in the test, and fail if it isn't catched. >>> try:
... do_something_that_raises_exception()
... raise Assertionerror("Exception Blah was not raised")
... except Blah:
... pass Ugly, yes, but easy. To make it less ugly you can make a "assertRaises()" like the one that exists on standard unit tests and call that. Not so ugly. |
Thank you for the suggestion but in my mind that's not a viable workaround, and not just because of uglyness: I'm using doctest to validate code examples, which are included in the documentation and are meant to be educational. If I'd change my examples to match the pattern you suggest they might still serve their purpose as a test but they'd become useless as an example. So it appears there is no real workaround for this issue. Any chance we can get the patch into 2.7? By the way, I said earlier that Barry suggested the ELLIPSIS workaround but it was actually ncoghlan who did so - apologies for the confusion. |
Hmm, wait. Here's a variation of your suggestion that works OK-ish even as an example: >>> try:
... # ... code that fails ...
... except mypkg.MyException, e:
... print(str(e))
Expected error message. This works because it omits the exception type in the output. It's still far from ideal, because as an example it's more complicated than it would need to be, but I guess it works as a stop-gap solution. Still, +1 for including the patch. |
The corner case I was talking about was the one where you actually *want* the old, more restrictive behaviour (i.e. you specifically want to receive 'x.y.Exception' and receiving 'a.b.Exception' instead should fail), but still want to ignore the details of the exception string representation. With this change in place, that corner case could be handled fairly easily by using the ELLIPSIS option instead of IGNORE_EXCEPTION_DETAIL (or else mandating the exception details as well the type). This change trips my "feature" meter, so it's probably too late for 2.7 (adding Benjamin to confirm), but definitely a good candidate for 3.2 later in the year. |
I think this one is worth making an exception for, since it would mean that a project could have 3.x doctests that also work with 2.7, whereas if we leave it out of 2.7 the doctests have to stay in 2.x format even if the project has (at some future point) dropped support for all earlier versions of 2.x. (Unless 3to2 can reformat Exceptions in doctest output?) |
It's not possible for 2to3 to reformat exceptions, as the formatting would need to go from TheException to themodule.TheException, and there is no way to figure out the module name... |
With a little more thought, I'm actually keen on including it as well (although the docs still need a bit more tweaking). The 2.x/3.x compatibility point is a good one. If Benjamin OKs it, I'll include this in the list of things I want to get to for beta2 (said list seems to be getting longer rather than shorter, but...) |
@lennart: no, in that direction (2.7 to 3.x) there's less of a problem. You leave the module name off in the doctest, and have 2to3 add the IGNORE_EXCEPTION_DETAIL to the doctest during translation. I was looking at the farther future case, where a project has moved to 3.x, but is still supporting 2.7 by using *3to2*. 3to2 could theoretically peel off the IGNORE_EXCEPTION_DETAIL and the module name, but I know that 2to3 doesn't touch the *output* of doctests, so I'm thinking that 3to2 probably doesn't either. (Maybe it could, for the limited case of Exceptions, but that seems like a big enough project to motivate including this patch in 2.7.) |
Sure, but +IGNORE_EXCEPTION_DETAIL will only work on Python 3.2+, so 2to3 can't solve the issue. It can only help once 3.2 does the actual solving. ;) 3to2 could simply remove the module name from exceptions in the output. You don't need to touch IGNORE_EXCEPTION_DETAIL for that. |
By that logic, 2to3 can't solve anything. I don't think there's any question that this patch should be applied to 3.2. 3.1 might be an issue as it is a new feature, but maybe we can claim it is a bug fix :) As for 3to2, like I said I don't think 3to2 touches doctest *output*, so removing the module name would be require the addition of a whole new feature (IIUC). |
2010/4/15 Nick Coghlan <report@bugs.python.org>:
I'm ok with it if you or someone else takes care of it. |
Committed for 2.7 in r80578 I'll forward port to 3.2 at some point after the next 2.7 beta is out. |
And done for 3.2 in r81944 (that checkin included a correction to the docs example which I backported to 2.7 in r81945) |
I take it the IGNORE_EXCEPTION_DETAIL should ignore the module name Is there a separate bug to enhance 2to3 to turn IGNORE_EXCEPTION_DETAIL |
On Wed, Jul 28, 2010 at 11:40 PM, Peter <report@bugs.python.org> wrote:
Correct (it's a new feature rather than a bug fix)
That would be a separate request, but I'm not sure it is even feasible |
2to3 can convert doctests, it just can't convert the *output* portion of doctests. because they are arbitrary strings and not syntactically valid Python code. Since turning on this flag would require recognizing something in the output portion of the doctest (which 2to3 doesn't handle), it can't be turned on by 2to3. On the other hand, it seems like it wouldn't be too hard to write a special purpose script to handle this specific case...since I don't know 2to3, I don't know how hard it would be to integrate such a special purpose script. (I do remember Benjamin saying 2to3 really ought to have a plugin system...so my guess is the answer is "not too easy"). In any case, as Nick said, that would be a separate RFE, and will likely get nowhere without someone volunteering to write the script/patch. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: