[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Martin Panter added the comment:
Robert: in issue17911-2.patch (and the eventual commit) you added a check for
value == 'None' in _format_final_exc_line(). Why was this necessary? I think it
is the cause of my Issue 27348 regression:
>>> traceback.format_exception(Exception, Exception("One"), None)
['Exception: One\n']
>>> traceback.format_exception(Exception, Exception("None"), None)
['Exception\n']
--
nosy: +martin.panter
___
Python tracker
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Regression fixed AFAICT, please re-open if not. -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Looking at the regression now. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Terry J. Reedy added the comment: Idle is partially disabled in a2 (#23633), with symptoms similar to that of the first example in msg237230. Trivial test: open Idle Shell, enter '1/0'. If this is not fixed before .a3, I think it should be reverted until it is. -- nosy: +terry.reedy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Changes by Terry J. Reedy tjre...@udel.edu: -- Removed message: http://bugs.python.org/msg237840 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Terry J. Reedy added the comment: (correct new issue #) Idle is partially disabled in a2 (#23631), with symptoms similar to that of the first example in msg237230. Trivial test: open Idle Shell, enter '1/0'. If this is not fixed before .a3, I think it should be reverted until it is. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Roundup Robot added the comment: New changeset 648b35f22b91 by Berker Peksag in branch 'default': Issue #17911: Tweak traceback documentation. https://hg.python.org/cpython/rev/648b35f22b91 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Ok, cgitb - its passing a string instead of an exception type into format_exception - something that was never supported - it only works by accident AFAICT, because the old format code was ignoring the etype - it was deriving the type from the value. Thats easy to accomodate in the compat code. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Ned Deily added the comment: Much better, thanks! -- priority: critical - normal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Roundup Robot added the comment: New changeset 7cea10917f40 by Robert Collins in branch 'default': Fix brownbag in issue 17911 commit https://hg.python.org/cpython/rev/7cea10917f40 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Thats really strange, I did a ./python -m test run before committing - the brown bag was due to me running with the patch for 22936 also applied. Looking into the failures reported now. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Fixes for buildbots. -- Added file: http://bugs.python.org/file38335/issue-19711-8.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
STINNER Victor added the comment: There are failures on buildbot. http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%203.x/builds/5651/steps/test/logs/stdio http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5783/steps/test/logs/stdio http://buildbot.python.org/all/builders/PPC64%20PowerLinux%203.x/builds/3109/steps/test/logs/stdio Examples: == ERROR: test_cause_tb (test.test_code_module.TestInteractiveConsole) -- Traceback (most recent call last): AttributeError The above exception was the direct cause of the following exception: Traceback (most recent call last): File /opt/python/3.x.langa-ubuntu/build/Lib/code.py, line 91, in runcode exec(code, self.locals) File console, line 1, in module ValueError During handling of the above exception, another exception occurred: Traceback (most recent call last): File /opt/python/3.x.langa-ubuntu/build/Lib/test/test_code_module.py, line 85, in test_cause_tb self.console.interact() File /opt/python/3.x.langa-ubuntu/build/Lib/code.py, line 245, in interact more = self.push(line) File /opt/python/3.x.langa-ubuntu/build/Lib/code.py, line 267, in push more = self.runsource(source, self.filename) File /opt/python/3.x.langa-ubuntu/build/Lib/code.py, line 75, in runsource self.runcode(code) File /opt/python/3.x.langa-ubuntu/build/Lib/code.py, line 95, in runcode self.showtraceback() File /opt/python/3.x.langa-ubuntu/build/Lib/code.py, line 144, in showtraceback for value, tb in traceback._iter_chain(*ei[1:]): AttributeError: module 'traceback' has no attribute '_iter_chain' == ERROR: test_html (test.test_cgitb.TestCgitb) -- Traceback (most recent call last): File /home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/test/test_cgitb.py, line 23, in test_html raise ValueError(Hello World) ValueError: Hello World During handling of the above exception, another exception occurred: Traceback (most recent call last): File /home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/test/test_cgitb.py, line 27, in test_html html = cgitb.html(sys.exc_info()) File /home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/cgitb.py, line 189, in html ''.join(traceback.format_exception(etype, evalue, etb))) File /home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/traceback.py, line 109, in format_exception etype, value, tb, limit=limit).format(chain=chain)) File /home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/traceback.py, line 433, in __init__ if exc_type and issubclass(exc_type, SyntaxError): TypeError: issubclass() arg 1 must be a class -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Also apologies - ned told me on IRC that python -m test -uall is needed, not just -m test. Doh. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: code module is using a _function from within the traceback module to filter out a frame - we can do that with the new interface easily (it filters the first tem). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: The decimal failure is due to _format_final_exc_line now filtering out 'None' as well, because the string captured values of objects leads to None - 'None' before we do rendering. I think in this case its reasonable to change the behaviour (since None itself was already filtered out, its only the special case of a value of 'None' that worked previously). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Roundup Robot added the comment: New changeset 73afda5a4e4c by Robert Collins in branch 'default': Issue #17911: traceback module overhaul https://hg.python.org/cpython/rev/73afda5a4e4c -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Ned Deily added the comment: With 7cea10917f40 applied, multiple tests are now failing on most buildbots. For example, on OS X 10.10, test_cgitb test_code_module test_decimal: test_blanks (test.test_cgitb.TestCgitb) ... ok test_fonts (test.test_cgitb.TestCgitb) ... ok test_html (test.test_cgitb.TestCgitb) ... ERROR test_syshook_no_logdir_default_format (test.test_cgitb.TestCgitb) ... ok test_syshook_no_logdir_text_format (test.test_cgitb.TestCgitb) ... ok test_text (test.test_cgitb.TestCgitb) ... ERROR test_banner (test.test_code_module.TestInteractiveConsole) ... ok test_cause_tb (test.test_code_module.TestInteractiveConsole) ... ERROR test_console_stderr (test.test_code_module.TestInteractiveConsole) ... ok test_context_tb (test.test_code_module.TestInteractiveConsole) ... ERROR test_ps1 (test.test_code_module.TestInteractiveConsole) ... ok test_ps2 (test.test_code_module.TestInteractiveConsole) ... ok test_syntax_error (test.test_code_module.TestInteractiveConsole) ... ERROR test_sysexcepthook (test.test_code_module.TestInteractiveConsole) ... ERROR ** 1 items had failures: 1 of 4 in decimal.Context.create_decimal_from_float See also, for example, the Debian buildbot where, in addition, test_fractions failed: http://buildbot.python.org/all/builders/AMD64%20Debian%20PGO%203.x/builds/140/steps/test/logs/stdio -- nosy: +ned.deily priority: normal - critical ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Ok, all changes applied, lets see how this looks to folk. -- Added file: http://bugs.python.org/file38324/issue17911-5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Changes by Robert Collins robe...@robertcollins.net: Added file: http://bugs.python.org/file38325/issue17911-6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: @Mahmoud thanks! I had a quick look and the structural approach we've taken is a bit different. What do you think of the current patch here (issue17911-4.patch) ? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Mahmoud Hashemi added the comment: Hey all, great to see this being worked on so diligently for so long. Having worked in this area for a while (at home and at PayPal), we've got a few learnings to share: 1) linecache is textbook not-threadsafe. For example, https://hg.python.org/cpython/file/default/Lib/linecache.py#l38 For a lightweight traceback wrapper to be concurrency-friendly, we've had to catch KeyErrors, like so: https://github.com/mahmoud/boltons/blob/master/boltons/tbutils.py#L115 It's kind of a blanket approach, but maybe we could make a separate issue and help out with a linecache refresh? 2) We use something like (filename, lineno) in our DeferredLine class, but for very lightweight areas (e.g., greenlet creation) we just save a reference to the code object, as the additional attribute accesses do end up showing up in the profiles. 3) Generally we've found the APIs in TracebackInfo here to be pretty sufficient/functional: https://github.com/mahmoud/boltons/blob/master/boltons/tbutils.py#L134 Let me know if you've got any questions on that, and keep up the good work! -- nosy: +mahmoud ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: I'm idealogically opposed to polymorphic interpretation of args :) Antoine, will you be ok with one __init__ and one classmethod? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Sorry xonatius I wasn't clear: AFAICT your patch is going to require changes to the traceback tests, and this issue is changing the implementation substantially: I was suggesting that you make sure your patch applies on top of this issue, not that you merge them :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Nick, Antoine - I'm now stuck between you. Options going forward: - I can JFDI realising you won't both be happy :) - you two can reach consensus! I could cripple __init__ by switching to __new__, but I think thats massive overcomplication and not needed. I can of course add more text to the pydoc prose saying 'do not use __init__' if that would address Nicks concern. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: Escalating to python-dev for API design feedback is the usual path forward if we reach an impasse in the tracker comments. I'll make one more attempt at persuading Antoine here though: the fact that we're being tempted to add do not use this API the way you would normally expect to use a Python API, even though it works exactly as you might expect to the docs is a big red flag for me :) There's a split between the low level API that exposes implementation details (the exception state triple) and high level API that hides them in the current patch, and Antoine is entirely correct that we previously omitted the latter. It doesn't follow for me that we should also hide the fact that the higher level convenience API is implemented in terms of the lower level more implementation oriented one. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: You can also switch back to a single constructor, taking either an exception instance or an exception triple. That way everyone is happy - except perhaps if you are ideologically opposed to polymorphic signatures :-) Switching to python-dev would probably be overkill for this. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Daniil Bondarev added the comment: As suggested adding patch from http://bugs.python.org/issue2786 which prints qualnames in function call exceptions. e.g: class A: ... def __init__(self): ... pass A(1) Traceback (most recent call last): File stdin, line 1, in module TypeError: A.__init__() takes 1 positional argument but 2 were given -- nosy: +xonatius Added file: http://bugs.python.org/file38034/qualname_in_exceptions.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: It's genuinely user hostile, as the from_exc_tuple version is entirely redundant, and folks using pydoc to discover the API won't know they shouldn't use the constructor directly. That means both forms will get used, but only one will be documented. When people ask why a second spelling for the lower level API was added, to make it look like it exists at the same level as the higher level convenience API isn't going to be a satisfying explanation. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: Le 06/02/2015 09:34, Nick Coghlan a écrit : It's genuinely user hostile, as the from_exc_tuple version is entirely redundant, and folks using pydoc to discover the API won't know they shouldn't use the constructor directly. Using pydoc isn't generally a successful way of discovering an API. It shows you too many details that don't matter. Besides our docstrings are much terser and less informative than the Sphinx doc. pydoc is useful mostly as a Python-specific man command: you know something exists, just want to remember the details. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: dir() will get me TracebackException as a name, help(traceback.TracebackException) will get me its signature. IDEs with autocomplete and signature tooltips will do the same. There is nothing in those usage sequences to say don't use __init__, use this redundant class method with the same signature instead. I agree providing a directly from an exception constructor is essential for cases where you're working with a caught exception at the Python level. I don't agree with the idea of duplicating the required low level API under a different name so it doesn't *look* like a lower level API in the documentation. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: I just posted some review comments. I also realized that it posted some older comments that I hadn't submitted before... some of them may be obsolete, sorry :-/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: I think in this case, the fact that it's easier to decompose an exception into the corresponding triple rather than vice-versa, together with the fact that other exception state manipulation APIs like exc_info() and __exit__() methods work with triples, means it makes sense to accept the triple as the base constuctor API, and then have the convenience method to destructure a caught exception. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: The solution of having two constructor classmethods actually seems nice and symmetric to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: +1 for having the base __init__ API be the exception triple and then a TracebackException.from_exception() class method as an alternate constructor. However, the redundancy between TracebackException.__init__() and TracebackException.from_exc_tuple() feels very strange to me. It leaves the __init__ working-but-undocumented and isn't a pattern I can recall seeing anywhere else in the standard library. Would it be possible to just add the from_exception() class method and leave the documented (and only) API for building from a triple being the normal constructor call? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: I can certainly do that; I was aiming to make it fair I guess, given Antoines strong feelings on this matter. As long as I'm not piggy-in-the-middle, I don't have a care in this regard :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: This iteration provides two constructors for TracebackException, one for exception objects, one for exc_info tuples. So it should be easy to use. The __init__ takes the exc_info tuple because thats less code (much easier to destructure rather than restructure), but in the docs and docstrings I point to the classmethods, which includes from_exception - that takes a single exception object. -- Added file: http://bugs.python.org/file38007/issue17911-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: It's tedious and complicated compared to the natural way. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: Most of the time when I'm working heavily with exceptions it's something related to contextlib, so I'm likely getting my exception details from either sys.exc_info() or the arguments to __exit__. That means I start out with an exception triple, and the only time I need to look at type(exc) or exc.__traceback__ is when I'm following exception chains. However, Antoine's right that if you got your exception from an *except clause* rather than one of the more indirect APIs, then you're just going to have the exception, rather than an exception triple. So I think that's where the argument for accepting exception-or-triple comes from: handling an exception directly is for use with except clauses, while handling triples is convenient for use with sys.exc_info(), __exit__ methods and for general backwards compatibility. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: tb = TracebackException(*sys.exc_info, lookup_lines=False) You don't need an exception tuple in Python 3, the exception instance is enough. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Changes by Martin Richard mart...@martiusweb.net: -- nosy: +martius ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: I put some detailed comments in the review, but my main large scale concern is with the traceback.Frame name. Making frame object an ambiguous phrase seems like a bad idea to me, so I've suggested making it FrameSummary (or FrameInfo, but on reflection I prefer the more explicit term). That would then suggest also making it traceback.StackSummary for consistency. I think having the top level API be an iterable would be OK, but may be a little surprising. Is there a major advantage to avoiding building the whole traceback in memory? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: Le 26/01/2015 19:36, Robert Collins a écrit : Two key notes: the use of an exception triple is useful to ease backports of this Then I'd recommend making it exception-or-tuple (rather than a *exc_info), so that both situations are satisfied. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Thanks for the review - shall action it all as it seems all good improvements. Two key notes: the use of an exception triple is useful to ease backports of this: a primary goal for me is being able to use the locals stuff in unittest for existing production code bases like OpenStack: there's a balance to be struck between new shiny features like __traceback__ and backporting. I'll consider the idea of value-or-triple; if all the consuming code is going to be using the triple interface I'm not sure the value one would, drumroll please, have value. The locals stuff is as you note not complete or exposed yet. I wanted it in the design to be able to vet that it will actually be feasible without a later API break. I'm not -quite- there but I do also agree that it doesn't need to be in the output layer yet (in fact there's a different ticket for that anyway). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: And for profit, review changes applied (minus the small number I disagreed with). I've clarified in the code why the exc_info tuple break out is still used (compat with the legacy API is the strongest argument). I haven't fleshed out the locals thing properly yet, I'm going to do one more iteration on that then ask for a final review before committing. (Unless we're in freeze ATM ?) -- Added file: http://bugs.python.org/file37870/issue17911-3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: actually, strike that, I'm happy with this pending a final +1 from another reviewer. Finishing the locals stuff is a separate patch, and will look like a single new parameter to StackSummary.extract, similarly on TracebackException.__init__ and then change the format logic to show it. @Antoine - I'm not super happy with inspecting the first arg to do type-or-value stuff, if you feel its a super issue we can do it, but I'd rather wait and add it once we've got some feedback. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: The generator thing: the code was refactoring not so long ago to be generator based internally with lists as a UI shim. I don't think there is a major advantage either way. Less buffering on one hand. Less convenient in some cases on the other. Simple use like ''.join(foo.format()) is going to be unaffected. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Why do you consider it crippling? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: On the triple-or-value front, the main case I see for considering the you don't need exception state triples any more argument already lost is the fact exc_info still returns a triple, and __exit__ methods still accept them as unpacked arguments. The C level exception handling APIs are also all still exception state triple based. In this view, __traceback__ is primarily about accessing *chained* tracebacks, while the top-level APIs still focus on exception state triples. This perspective also has the virtue of confining use of the Python 3 only exception traceback attribute to code that is taking advantage of a Python 3 only feature (i.e. exception chaining). For chaining independent exception state handling, working with exception state triples then remains the one obvious way to do it, a way which is conveniently also source compatible between Python 2 and 3. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: In this view, __traceback__ is primarily about accessing *chained* tracebacks Please let's not make up sophistic arguments when the only motivation for triples is backwards compatibility of *existing* APIs. Exception triples are an unnecessary bother in Python 3. This new API should accept exception instances primarily. Accepting triples as well is ok but it shouldn't be made the only possibility. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: So its fairly simple IMO: it will be more code, not less, to support the non-triple API, *and* it can be added later, unless we're proposing not to support the triple API at all (which hasn't been proposed AFAICT). To me thats a fairly strong argument for adding non-triple support later. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Antoine Pitrou added the comment: Le 27/01/2015 00:30, Robert Collins a écrit : Robert Collins added the comment: So its fairly simple IMO: it will be more code, not less, to support the non-triple API, *and* it can be added later, unless we're proposing not to support the triple API at all (which hasn't been proposed AFAICT). I am proposing it. Passing a single exception instance is the natural way of expressing such an API. Crippling Python 3 APIs for the purpose of backwards compatibility is not ok. Nobody wants to write some_func(exc.__class__, exc, exc.__traceback__) when they could simply write some_func(exc). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Right, and a usable API. I believe that this will meet Guido's use case: tb = TracebackException(*sys.exc_info, lookup_lines=False) some time later if should_show_tb: lines = list(tb.format()) I'm not 100% sold on the public API being a generator, but rather than forcing it one way or the other, I'll let reviewers tell me what they think :) Performance wise, this is better, with the following times: format_stack - 5.19ms new API to extract and not format - 3.06ms new API to extract, not lookup lines and not format - 2.32ms Formatting is then 2-3ms per 500 line traceback actually formatted, which seems terribly slow, but its still better than the 8+ms trunk takes (see my earlier tests). I'll look at tuning the time to render an actual trace later, since I don't like paying high costs in unittest ;) - but AIUI this should be enough to help asyncio as is. Updated test script I used to isolate times with timeit: import traceback def recurse(count, lookup_lines=True): if count 0: return recurse(count - 1, lookup_lines=lookup_lines) if lookup_lines: return traceback.Stack.extract(traceback.walk_stack(None), lookup_lines=True) else: return traceback.Stack.extract(traceback.walk_stack(None), lookup_lines=False) def doit(): len(recurse(500)) def doit_lazy(): len(recurse(500, False)) -- Added file: http://bugs.python.org/file37862/issue17911-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Stack and Frame looking good, next update will be next Monday, when I finish off my TracebackException class. -- Added file: http://bugs.python.org/file37782/issue17911-1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: I've split out the stat question to http://bugs.python.org/issue23273 - we can optimise it slightly in this patch, but I think its scope creep here, and will be unclear, to dive after a full fix in this issue. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Changes by Robert Collins robe...@robertcollins.net: Added file: http://bugs.python.org/file37769/linecache_2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: This is a WIP patch, including it to just share progress. -- Added file: http://bugs.python.org/file37770/frame_1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: And this improves the scaling of the cache stating overhead, but we may well want to fix this more fundamentally - e.g. by linking the cache ownership into the import system somehow, so that when a file is reimported, the cached source is automatically evicted, and format_stack etc have no need to stat at all. I tested this with the following script in timeit: import traceback def recurse(count): if count 0: return recurse(count - 1) return traceback.format_stack() def doit(): len(recurse(500)) -- Added file: http://bugs.python.org/file37771/tb_stats_1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: One thing I noticed while working on this, traceback today stats each included file once per frame per call into e.g. format_list. This might actually account for quite some of the overhead. I'll include an optimisation for this in my new API. http://paste.openstack.org/show/158714/ http://paste.openstack.org/show/158715/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: Ok, here's a draft patch for linecache. Next up, poking around the new TB API. -- Added file: http://bugs.python.org/file37700/linecache_1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: w.r.t. a new linecache interface, it looks like we need two attributes from f_globals: __name__ and __loader__, so that we can eventually call __loader__.get_source(__name__). One small change (to let me focus on traceback) would be to add another kw argument to the existing calls that take module_globals, called e.g. get_source, which would be a simple callable (source = get_source()). For the deferred linecache situation, we'd then create partial(f_globals.__loader__.get_source, f_globals.__name__) and keep that around until we need the source. We could even abstract that out into a function in linecache to keep the knowledge in one place. Another way to tackle this would be to add a new function to linecache that lazily seeds the cache: it would stash the get_source method and the name against the filename, and when getline[s] is called actually invoke it. I think the second way is a bit nicer myself. What do folk think? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Robert Collins added the comment: I'll put something together. -- nosy: +rbcollins ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Nick Coghlan added the comment: Belatedly looking at this again: one thing I noticed is that the summary currently loses info if __cause__ is set. It would be better to follow the PEP 409/415 model and report the __suppress_context__ flag as part of the summary, rather than dropping the context information. That allows the decision on whether or not to show the context information to be deferred to display time, rather than discarding it eagerly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17911] traceback: add a new thin class storing a traceback without storing local variables
Changes by STINNER Victor victor.stin...@gmail.com: -- title: Extracting tracebacks does too much work - traceback: add a new thin class storing a traceback without storing local variables ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17911 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
