Possible implementation of negative limit for traceback functions

[vaultah]

BPO	22619
Nosy	@terryjreedy, @rbtcollins, @bitdancer, @serhiy-storchaka, @vaultah
Files	9f618a0c2880.diff 9cb7aaad1d85.diff traceback_rev.diff traceback_rev_fixed.diff traceback_rev2.diff traceback_rev3.diff traceback_negative_limit_2.patch traceback_negative_limit_3.patch traceback_limit_doc.diff traceback_limit_doc_rev2.diff traceback_limit_doc_rev3.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/serhiy-storchaka'
closed_at = <Date 2015-05-03.10:47:40.298>
created_at = <Date 2014-10-12.15:24:10.881>
labels = ['type-feature', 'library']
title = 'Possible implementation of negative limit for traceback functions'
updated_at = <Date 2015-05-03.10:47:40.297>
user = 'https://github.com/vaultah'

bugs.python.org fields:

activity = <Date 2015-05-03.10:47:40.297>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-05-03.10:47:40.298>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2014-10-12.15:24:10.881>
creator = 'vaultah'
dependencies = []
files = ['36891', '36899', '37600', '37615', '37628', '37632', '37731', '38622', '39152', '39162', '39166']
hgrepos = []
issue_num = 22619
keywords = ['patch']
message_count = 35.0
messages = ['229167', '229168', '229173', '229175', '229180', '229183', '229247', '229312', '229887', '229888', '229895', '230660', '231047', '231902', '231975', '233294', '233450', '233541', '233551', '233575', '233592', '233597', '234141', '236453', '238780', '238785', '238800', '241559', '241565', '241684', '241731', '241739', '242092', '242459', '242460']
nosy_count = 7.0
nosy_names = ['terry.reedy', 'rbcollins', 'r.david.murray', 'python-dev', 'serhiy.storchaka', 'vaultah', 'flwaultah']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22619'
versions = ['Python 3.5']

[vaultah]

This is the possible patch for this proposal: https://mail.python.org/pipermail/python-ideas/2014-October/029826.html.

[vaultah]

def _extract_tb_or_stack_iter(curr, limit, extractor):
    # Distinguish frames from tracebacks (need to import types)
    if limit is None:
        limit = getattr(sys, 'tracebacklimit', None)
    elif limit < 0 and isinstance(curr, types.TracebackType):
        seq = list(_extract_tb_or_stack_iter(curr, None, extractor))
        yield from seq[limit:]
    else:
        pass

[serhiy-storchaka]

You forgot a patch.

[vaultah]

Sorry, first time posting here; accidently pressed Enter.

[serhiy-storchaka]

I have added comments on Retvield (a system for patch reviewing). You should receive an email about this.

In general the patch is not optimal. There is no need to load source lines for dropped entries.

[vaultah]

Thanks. I replied to your comments at Retvield. I'll update the patch tomorrow.

[vaultah]

Here's the updated (optimized) patch

[vaultah]

Renamed the cond variable, added tuple unpacking instead of using a single variable.

[vaultah]

I think the reason this patch hasn't been discussed well is that it only changes the behavior for traceback.*_tb functions that only deal with tracebacks. I commented on the review page that we don't have to change the behavior of traceback.*_stack functions to make it obvious. Let me show an example:

import sys

def a():
    b()

def b():
    c()

def c():
    d()

def d():
    def e():
        print_stack(limit=2) # Last 2 entries
        ''' Output:
            File "file", line 331, in d                                                                                                                                                                                       
              e()                                                                                                                                                                                                                         
            File "file", line 328, in e
              print_stack(limit=2) # Last 2 entries '''
        raise Exception
    e()
    
try:
    a()
except Exception:
    print_exc(limit=2) # 2 entries from the caller
    ''' Output:
        Traceback (most recent call last):
          File "file", line 336, in <module>
            a()
          File "file", line 318, in a
            b()
        Exception '''

If we decide to unify the behavior of *_tb and *_stack functions, the change will break some existing code, although the breakage will be purely cosmetic.

[bitdancer]

More likely the lack of discussion is just that Serhiy is busy :)

Breaking code is to be avoided if possible. Can you give an example of the "cosmetic" change?

I haven't fully reviewed the patch, but a more meaningful name than 'condition' might make the code more readable. Perhaps 'handling_negative_limit'?

[serhiy-storchaka]

Last patch is more complicated and needs more time to review.

I suppose the code would be more clear if split _extract_tb_or_stack_iter() to parts. One generator just generates (filename, lineno, name, f.f_globals) tuples. Then these tuples either handled directly or passed through a deque with fixed size.

And some tests would be good.

[vaultah]

I'm not sure what would be the best way to support negative limit for stack functions. Actually, I think the current behavior is not intuitive. For example we could make positive limit mean the same thing for traceback and stack functions (load N entries, starting from the caller), but in that case the change will inverse the behavior of code that uses stack functions. Since the traceback module is mostly used for printing/formatting the difference won't be crucial.

[vaultah]

I updated the patch.