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

IGNORE_EXCEPTION_DETAIL should ignore the module name #51739

Closed
regebro mannequin opened this issue Dec 13, 2009 · 28 comments
Closed

IGNORE_EXCEPTION_DETAIL should ignore the module name #51739

regebro mannequin opened this issue Dec 13, 2009 · 28 comments
Assignees
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@regebro
Copy link
Mannequin

regebro mannequin commented Dec 13, 2009

BPO 7490
Nosy @warsaw, @birkenfeld, @ncoghlan, @benjaminp, @regebro, @bitdancer, @peterjc
Files
  • python-trunk-exception-detail.diff
  • python-py3k-exception-detail.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/ncoghlan'
    closed_at = <Date 2010-06-12.13:48:46.976>
    created_at = <Date 2009-12-13.11:11:41.322>
    labels = ['type-bug', 'tests']
    title = 'IGNORE_EXCEPTION_DETAIL should ignore the module name'
    updated_at = <Date 2010-07-29.03:43:53.573>
    user = 'https://github.com/regebro'

    bugs.python.org fields:

    activity = <Date 2010-07-29.03:43:53.573>
    actor = 'r.david.murray'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2010-06-12.13:48:46.976>
    closer = 'ncoghlan'
    components = ['Tests']
    creation = <Date 2009-12-13.11:11:41.322>
    creator = 'lregebro'
    dependencies = []
    files = ['15972', '15973']
    hgrepos = []
    issue_num = 7490
    keywords = ['patch']
    message_count = 28.0
    messages = ['96329', '96366', '96367', '96369', '96370', '96439', '98147', '98149', '103191', '103194', '103195', '103196', '103197', '103198', '103199', '103211', '103216', '103217', '103218', '103222', '103223', '103229', '103253', '104432', '107654', '111807', '111865', '111888']
    nosy_count = 8.0
    nosy_names = ['barry', 'georg.brandl', 'ncoghlan', 'benjamin.peterson', 'lregebro', 'r.david.murray', 'Julian.Scheid', 'maubp']
    pr_nums = []
    priority = 'high'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue7490'
    versions = ['Python 2.7', 'Python 3.2']

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Dec 13, 2009

    In Python 3.x [1] the exception formatting prints the module path, while
    under 2.x it prints only the exception class name. This makes it very
    tricky to make doctests that pass under both Python 2 and Python 3
    without resorting to ugly tricks.

    Since IGNORE_EXCEPTION_DETAIL was implemented to hide differences
    between exception messages in 2.3 and 2.4, it was suggested on
    python-dev [2] that IGNORE_EXCEPTION_DETAIL should be extended to also
    ignore the module name, so that module.name.ExceptionClass and
    ExceptionClass would match each other. This is easily done by just
    changing the regexp that is done for matching.

    I'll attach diffs both for trunk and for py3k-branch, so that both forms
    can be used on both versions. The diffs include tests and suggested
    documentation changes (although I reserve the right to be useless at
    writing documentation).

    [1] And possibly in some cases under Python 2.7 according to reports in
    the thread on python-dev about this issue, although I haven't been able
    to confirm this. I'll include a 2.7 diff anyway, as it would be good if
    both syntaxes work under both versions, if people start using 3to2, for
    example.

    [2] http://mail.python.org/pipermail/python-dev/2009-December/094460.html

    @regebro regebro mannequin added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Dec 13, 2009
    @ncoghlan
    Copy link
    Contributor

    The only design level question I can see is as follows:

    ExceptionName matches ExceptionName (always)
    a.b.ExceptionName matches ExceptionName (under IGNORE_EXCEPTION_DETAIL)
    ExceptionName matches a.b.ExceptionName (under IGNORE_EXCEPTION_DETAIL)
    a.b.ExceptionName matches x.y.ExceptionName (???)

    Should that 4th case still match under IGNORE_EXCEPTION_DETAIL? My
    personal inclination is that it should match, but figured the point was
    worth discussing explicitly.

    The main reason I think it should match is that it would allow
    reasonably graceful handling of module renames between 2.x and 3.x.

    @bitdancer
    Copy link
    Member

    My impression is that IGNORE_EXCEPTION_DETAIL is designed to allow you
    to have a doctest as an example with a fully typed out exception detail,
    but have it pass even if the exception detail changes. If that is
    indeed the original design, then I think your case 4 should pass.]

    The one argument against it that I can see is the hypothetical case of
    an x.y.Error passing when the code actually raised an a.b.Error when a
    rename is *not* involved. But that seems like a marginal enough case
    that we could just ignore it. Especially since having case 4 pass makes
    the behavior of the modified IGNORE_EXCEPTION_DETAIL more consistent.

    @ncoghlan
    Copy link
    Contributor

    Agreed - particularly since that corner case can still be tested through
    doctest if desired by using ELLIPSIS instead of IGNORE_EXCEPTION_DETAIL.

    The patches mostly look good, but the doc changes should be updated to
    indicate that using ELLIPSIS doesn't handle the case of mismatched
    module names.

    E.g. """Note that :const:`ELLIPSIS` can also be used to ignore the
    details of the exception message, but such a test may still fail based
    on whether or not the module details are printed as part of the
    exception name. Using :const:`IGNORE_EXCEPTION_DETAIL` is also the only
    clear way to write a doctest that doesn't care about the exception
    detail yet continues to pass under Python releases prior to 2.4 (doctest
    directives appear to be comments to them)."""

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Dec 14, 2009

    Yes, x.y.Exception and a.b.Exception should match. I just realized I
    didn't add an explicit test for that, but maybe that's not strictly
    necessary.

    @warsaw
    Copy link
    Member

    warsaw commented Dec 15, 2009

    @lennart: yes, I do think you should add a test for that case. I
    haven't yet decided whether this should go into 2.6.

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Jan 22, 2010

    New diff for trunk, with the additional test

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Jan 22, 2010

    New diff for Py3k with the additional test

    @JulianScheid
    Copy link
    Mannequin

    JulianScheid mannequin commented Apr 15, 2010

    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

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Apr 15, 2010

    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.

    @JulianScheid
    Copy link
    Mannequin

    JulianScheid mannequin commented Apr 15, 2010

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

    @JulianScheid
    Copy link
    Mannequin

    JulianScheid mannequin commented Apr 15, 2010

    Ah, right... so there is no easy workaround at present?

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Apr 15, 2010

    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.

    @JulianScheid
    Copy link
    Mannequin

    JulianScheid mannequin commented Apr 15, 2010

    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.

    @JulianScheid
    Copy link
    Mannequin

    JulianScheid mannequin commented Apr 15, 2010

    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.

    @ncoghlan
    Copy link
    Contributor

    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.

    @bitdancer
    Copy link
    Member

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

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Apr 15, 2010

    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...

    @ncoghlan
    Copy link
    Contributor

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

    @bitdancer
    Copy link
    Member

    @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.)

    @regebro
    Copy link
    Mannequin Author

    regebro mannequin commented Apr 15, 2010

    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.

    @bitdancer
    Copy link
    Member

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

    @benjaminp
    Copy link
    Contributor

    2010/4/15 Nick Coghlan <report@bugs.python.org>:

    Nick Coghlan <ncoghlan@gmail.com> added the comment:

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

    I'm ok with it if you or someone else takes care of it.

    @ncoghlan ncoghlan self-assigned this Apr 16, 2010
    @ncoghlan
    Copy link
    Contributor

    Committed for 2.7 in r80578

    I'll forward port to 3.2 at some point after the next 2.7 beta is out.

    @ncoghlan
    Copy link
    Contributor

    And done for 3.2 in r81944 (that checkin included a correction to the docs example which I backported to 2.7 in r81945)

    @peterjc
    Copy link
    Mannequin

    peterjc mannequin commented Jul 28, 2010

    I take it the IGNORE_EXCEPTION_DETAIL should ignore the module name
    fix will not be applied to Python 3.1.x?

    Is there a separate bug to enhance 2to3 to turn IGNORE_EXCEPTION_DETAIL
    on?

    @ncoghlan
    Copy link
    Contributor

    On Wed, Jul 28, 2010 at 11:40 PM, Peter <report@bugs.python.org> wrote:

    Peter <p.j.a.cock@googlemail.com> added the comment:

    I take it the IGNORE_EXCEPTION_DETAIL should ignore the module name
    fix will not be applied to Python 3.1.x?

    Correct (it's a new feature rather than a bug fix)

    Is there a separate bug to enhance 2to3 to turn IGNORE_EXCEPTION_DETAIL
    on?

    That would be a separate request, but I'm not sure it is even feasible
    (doctests live inside strings, so I believe 2to3 has trouble fixing
    them - otherwise we would just get it to handle the exception renaming
    directly). Feel free to post an RFE though - the 2to3 folks can take a
    look at it.

    @bitdancer
    Copy link
    Member

    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.

    @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
    tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants