classification
Title: Expand traceback module API to accept just an exception as an argument
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, brett.cannon, cheryl.sabella, martin.panter, mbussonn, pablogsal, rhettinger, terry.reedy
Priority: low Keywords: patch

Created on 2016-02-18 22:59 by brett.cannon, last changed 2021-02-12 22:18 by terry.reedy. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 327 closed mbussonn, 2017-02-27 01:14
PR 22610 merged ZackerySpytz, 2020-10-09 05:57
Messages (19)
msg260485 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-02-18 22:59
When reading https://docs.python.org/3/library/traceback.html#traceback.print_exception I noticed that everything takes a traceback or a set of exception type, instance, and traceback. But since Python 3.0 all of that information is contained in a single exception object. So there's no reason to expand the APIs in the traceback module that take an exception to just take an exception instance and infer the exception type and grab the exception from exception instance itself.
msg260554 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-02-20 03:54
You title and post don't seem to match.  The title says to expand the API and the post says there is no reason to expand the API.  Did you mean 'good reason'?  Also, I think you meant 'grab the traceback' (from the exception) rather than 'grab the exception'.

How would you get from here to there and change required etype, value, tb to just required value, without breaking old code?
msg260559 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-20 05:12
There is precedent with Python 2’s “raise” statement for accepting an exception instance for the first parameter (where an exception class would otherwise be passed). Also, generator.throw() supports this; see Issue 14911 for clarifying its documentation.

I would support changing the following signatures so that the first parameter could hold the value, not just the type:

print_exception(etype, value=None, tb=None, limit=None, ...)
format_exception_only(etype, value=None)
format_exception(etype, value=None, tb=None, limit=None, ...)
TracebackException(exc_type, exc_value=None, exc_traceback=None, *, ...)
msg260571 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-02-20 19:48
So Terry's right that in my haste to write down the idea I contradicted myself. I do want to tweak the APIs in the traceback module to accept only an exception. The question is whether we need entirely new functions or if the pre-existing ones can be updated to work with just an exception.

And if we were to tweak the existing functions instead of add new ones, I would do it by making all arguments optional and adding a new keyword-only argument called `exc` that took the exception. Either `exc` would be set or the old arguments would need to all be set, and it would be an error to set both sets of arguments. And when the old arguments were taken away then all arguments for those functions would become keyword-only.
msg288615 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2017-02-27 00:18
Just came across that with wanting to use print_exception and got the same idea.

It seem like in print_exception, and format_exception, etype is already ignored and `type(value)` is used, so I think we could also "just" also ignore tb (unless set) and use `value.__traceback__`. Leaving the API to function(None, exception)

I don't see any clean way to migrate to a new API (IMHO a forced `exc kwarg` is not discoverable enough vs a (None, e, e.__traceback__)

A possibility, as `etype` is already unused, would be replace it with etype_or_exception, and in case it's a full exception use it as the sole parameter from which type and tb get extracted. 

Though that feels weird as well, and the Deprecation Cycles would need to be long.

Already emitting deprecation warnings (that etype is ignored) would be good.
msg288618 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2017-02-27 01:24
I've attempted a Pull-request trying to implement what Brett is describing. See https://github.com/python/cpython/pull/327, and opened http://bugs.python.org/issue29660 to document that etype is ignored and decide wether it should emit DeprecationWarning when used.
msg288621 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-02-27 07:31
Matthias’s proposal adds support for a new keyword-only “exc” argument:

    print_exception(exc=exception_instance)

I still think it is worth supporting a single positional argument as well:

    print_exception(exception_instance)

Another point is that it may be better to keep the existing parameter name “value”, rather than (eventually?) replacing it with “exc”. I think both of these things could be accomplished by juggling the “value” and “etype” parameters:

def print_exception(etype=None, value=None, tb=None, ...):
    if value is None:  # Assume value passed as first positional argument
        value = etype
        etype = None
    if etype is tb is None:  # Only value passed
        etype = type(value)
        tb = value.__traceback__
    # Existing code using (etype, value, tb)

The disadvantage of any of these changes is that we may want to maintain support for the old signature while Python 2 remains alive.
msg288645 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2017-02-27 15:44
> Another point is that it may be better to keep the existing parameter name “value”, rather than (eventually?) replacing it with “exc”. I think both of these things could be accomplished by juggling the “value” and “etype” parameters:

I agreed, and that what I would have done outside of CPython, but that felt like this kind of juggling was frowned upon. 

> The disadvantage of any of these changes is that we may want to maintain support for the old signature while Python 2 remains alive.

Your example does not seem to break the old signature. Or I fail to see how.

If that way would be accepted I'm happy to implement it.
msg288660 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-02-27 18:17
I don't think supporting both approaches is worth it; we should just choose one of them. As for which one, I'm torn. The single argument one is the most pragmatic, but changing types like has always bugged me. But as Martin points out, the `raise` syntax supports the class or an instance so there's precedent in regards to exceptions themselves. The parameter name is poorly named if we take it in for the first argument which is unfortunate.

Basically I can't decide. :)
msg288671 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-02-27 20:49
The print_exception API goes back to when exception values were (or at least, could be) strings.  Its doc is incomplete as to the requirements on the args and, at least by 3.5, erroneous.  To modify it, we need to understand how it actually works now.

In 2.7, 'etype' can apparently by anything with an __str__ method.  (Someone could check the 2.7 code.) At least Exception, 'abc', and None result in 'Exception', 'abc' or 'None' leading the last line.  Example:
>>> try: 1/0
except Exception as e:
	tb.print_exception(None, e, None)

None: integer division or modulo by zero

By at least 3.5, the etype arg can definitely be anything, because it is ignored.  The printed exception type is already grabbed from the exception. Any patch should not change this.  Note that this 3.x change already introduced an incompatibility.  In 3.5, the above prints 'ZeroDivisionError' instead of 'None'.

This holdover line in the doc "prints the exception etype and value after the stack trace" is wrong and should be corrected with a 'changed in 3.x' note if done after 3.0.  

In 2.7, value can at least be an exception, a string, or None (not documented).  (Ditto for checking the code.)

>>> try: 1/0
except Exception as e:
	tb.print_exception(None, 'zero-divive', tb=None)

None: zero-divive
>>> try: 1/0
except Exception as e:
	tb.print_exception(None, None, None)

None

In 3.?, value must be an exception instance (or compatible duck) (not documented).
...
  File "C:\Programs\Python36\lib\traceback.py", line 465, in __init__
    if (exc_value and exc_value.__cause__ is not None
AttributeError: 'str' object has no attribute '__cause__'

So, more potential incompatibilities with 2.x.

In 2.7, tb is needed to supply the traceback, or to suppress it.  As a separate parameter, it allows a traceback to be modified before printing.*  The option of a modified or omitted traceback (the ultimate modification) should be kept.

*IDLE edits tracebacks before printing to delete artifacts introduced by IDLE internals.  The attempt is to print what the console interpreter would.  I don't currently know whether it replaces the original on the exception or not.  There is also a proposal for the standard interpreter to edit tracebacks after recursion limit exceptions.  So traceback editing is useful, and I see no need to force replacement of the semi-private e.__traceback__.


My suggestions:

tb: In 3.7, in the API, change 'tb' to 'tb=True'.  If tb is left True, grab it from the exception.  If tb is explicitly supplied, use it.  If tb is set to False or (for back compatibility) None, suppress it. 

value: In 3.5+ document that it must be an exception.
(Optional) Change 'value' to 'exc' in the API to reflect the 3.x restriction. Document 'value' as a deprecated synonym for keyword usage. Keep the synonym until after 2.7. 

etype: In 3.5+ document that it is an ignored dummy argument and that one can just pass 0, '', or None.
(Optional) Deprecate the parameter and make it optional.  This can be handled* in the code and would be like range having option 'start'.  This is messy but would be temporary.  Remove after 2.7.

* 1 arg = exc/value, 3 args are etype, value, tb, 2 args are exc, tb or etype, exc depending on whether the type(first) is BaseException.


I am inclined to go with both options, but even if the 3 of us agree, I might be inclined to post our intention on pydev.
msg288673 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-02-27 21:03
Comments probably apply to format_exception and in part, format_exception_only (both also patched in PR237) but I have not checked behavior or code at all.
msg288857 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2017-03-03 05:29
> (Optional) Change 'value' to 'exc' in the API to reflect the 3.x restriction Keep the synonym until after 2.7. 

Do you mean after 2.7 EOL, or after 3.7 ?


> etype: In 3.5+ document that it is an ignored dummy argument and that one can just pass 0, '', or None.
> (Optional) Deprecate the parameter and make it optional.  This can be handled* in the code and would be like range having option 'start'.  This is messy but would be temporary.  

That seem hard to do if we want to allow func(exception), should we attempt to emit a deprecation warning only when etype is used in the Legacy Form ?

> Remove after 2.7.

Same question as above, unsure what you mean. 


> (Optional) Change 'value' to 'exc' in the API to reflect the 3.x restriction. Document 'value' as a deprecated synonym for keyword usage.

That seem like overly complicated as now the actual exception object can be either in etype, value, or exc. 


Let my try to give example to see if I understood correctly

2.7 compat

    print_exception(etype, value, tb)
    print_exception(etype=etype, value=value, tb=tb) 

3.5 compat
    print_exception(None, value, tb)
    print_exception(etype=None, value=value, tb=tb)
                                      
3.7
    print_exception(value)              == print_exception(type(value), value, value.__traceback__)
    print_exception(value, tb=True)     == print_exception(type(value), value, value.__traceback__)

    print_exception(value, tb=None)     == print_exception(type(value), value, None)
    print_exception(value, tb=False)    == print_exception(type(value), value, None)


    print_exception(value, tb=faketb)   == print_exception(type(value), value, faketb)
                                  
    # tb can be positional

    print_exception(value, tb) == print_exception(value, tb=tb)

Signature would thus be:

    if first param isinstance BaseException: # 3.7+, prefered form
        print_exception(exc, [tb ,] *, [limit, file, chain])
    else:
        print_exception(etype, value, tb, [limit, file, chain])
        # etype being ignored

Should exc be positional only ?

    print_exception(exc, /, [tb ,] *, [limit, file, chain])

Should `limit`, `file`, `chain` be allowed to be positional ?

    print_exception(exc, [tb , limit, file, chain])
msg288870 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-03-03 07:48
Matthias, I will try to respond tomorrow.
msg339681 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-04-08 23:04
The last comment on the original PR for this issue was to wait until an API was decided upon before proceeding with creating a new PR.  Bumping this issue to generate new discussion and hopefully reach concession on an API.
msg339885 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-04-10 19:33
Boy, having a postional-only parameter in that first position would have been handy when we created this API (as Matthias pointed out). :)

The 'exec' keyword-only parameter is obviously the safest option here.

Making the first parameter positional-only and then making the other parameters optional would lead to the most fluid API long-term as people I suspect would much rather just pass in an object than always specifying the keyword-only parameter every time going forward. I also doubt anyone is specifying etype by name.

So my vote is:

+1 to a positional-only first parameter
+0 to 'exc' keyword-only parameter
msg355021 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-10-20 20:49
@mbussonn, were you interested in continuing with Brett's (and your) suggestion for the API by making the first argument positional-only and the others optional?
msg356574 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2019-11-14 04:44
> were you interested in continuing with Brett's (and your) suggestion for the API by making the first argument positional-only and the others optional?

Yes, but I currently do not have the time; if someone else want to take over; or if you need to close this issue or corresponding PR for now please feel free to do so. I'll come back to it at some point.
msg356575 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-11-14 05:02
It would be nice to see this one come to fruition.
msg380438 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-11-05 22:18
New changeset 91e93794d5dd1aa91fbe142099c2955e0c4c1660 by Zackery Spytz in branch 'master':
bpo-26389: Allow passing an exception object in the traceback module (GH-22610)
https://github.com/python/cpython/commit/91e93794d5dd1aa91fbe142099c2955e0c4c1660
History
Date User Action Args
2021-02-12 22:18:58terry.reedysetversions: + Python 3.10, - Python 3.9
2021-01-08 13:12:48iritkatriellinkissue14655 superseder
2020-11-05 22:19:23pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-11-05 22:18:58pablogsalsetnosy: + pablogsal
messages: + msg380438
2020-10-09 05:57:12ZackerySpytzsetkeywords: + patch
nosy: + ZackerySpytz
pull_requests: + pull_request21593
2019-11-14 05:02:53rhettingersetnosy: + rhettinger
messages: + msg356575
2019-11-14 04:44:45mbussonnsetmessages: + msg356574
2019-10-20 20:49:13cheryl.sabellasetmessages: + msg355021
versions: + Python 3.9, - Python 3.8
2019-04-10 19:33:27brett.cannonsetmessages: + msg339885
2019-04-08 23:04:11cheryl.sabellasetnosy: + cheryl.sabella

messages: + msg339681
versions: + Python 3.8, - Python 3.7
2017-03-03 07:48:04terry.reedysetmessages: + msg288870
2017-03-03 05:30:00mbussonnsetmessages: + msg288857
2017-02-27 21:03:09terry.reedysetmessages: + msg288673
2017-02-27 20:49:37terry.reedysetmessages: + msg288671
2017-02-27 18:17:16brett.cannonsetmessages: + msg288660
2017-02-27 15:44:30mbussonnsetmessages: + msg288645
2017-02-27 07:31:36martin.pantersetstage: test needed -> patch review
messages: + msg288621
versions: + Python 3.7, - Python 3.6
2017-02-27 01:24:20mbussonnsetmessages: + msg288618
2017-02-27 01:14:23mbussonnsetpull_requests: + pull_request287
2017-02-27 00:18:55mbussonnsetnosy: + mbussonn
messages: + msg288615
2016-02-20 19:48:08brett.cannonsetmessages: + msg260571
2016-02-20 05:12:44martin.pantersettype: enhancement

messages: + msg260559
nosy: + martin.panter
2016-02-20 03:54:51terry.reedysetnosy: + terry.reedy
messages: + msg260554
2016-02-18 22:59:10brett.cannoncreate