classification
Title: tracemalloc.Traceback.format() should have an option to reverse the traceback
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jbakker, pitrou, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2017-11-23 20:10 by pitrou, last changed 2017-11-29 23:05 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4534 merged jbakker, 2017-11-24 00:06
Messages (9)
msg306856 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-11-23 20:10
Right now tracemalloc.Traceback.format() returns the frames in "most recent call first" order.  This is not intuitive since ordinary Python tracebacks are displayed in "most recent call last" order.  It would be nice to add a `reverse` argument to change that.
msg306859 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-23 21:29
The option seems reasonable. Would you like to work on a pull request?
msg306863 - (view) Author: Jesse Bakker (jbakker) * Date: 2017-11-24 00:08
I can work on this if you want. Can have a PR ready in a few minutes.
msg306885 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-24 10:53
tracemalloc.Traceback contains "raw" data from the C _tracemalloc module. But since we are at Python level, we are free to change how data is rendered to user.

Since Antoine Pitrou was surprised by the current frame order in Traceback.format() ("This is not intuitive since ordinary Python tracebacks are displayed in "most recent call last" order"), what do you think of adding a new parameter but also **reverse frames by default**?

Maybe we can go further and even reverse frames in Traceback constructor? This is a backward incompatible change, but I don't think that a lot of code in the wild uses the tracemalloc *API*.

About the parameter name: I'm ok with "reverse". *But* if we reverse by default, the "reverse" name can be confusing. Maybe we can use a less confusing name like "most_recent_last=True" (or "most_recent_first=False")?

--

Note: The faulthandler module also dumps starting with the "(most recent call first)", but I don't want to change that. It's for technical reason: faulthandler is called when something really bad happens and so Python internal structures may be corrupted. Moreover, frames is a single chained list, it's hard to iterate over them in the backward order (especially in faulthandler code where heap memory allocations are denied).
msg306886 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-11-24 10:57
> what do you think of adding a new parameter but also **reverse frames by default**?

This would be fine with me.  I didn't want to suggest it because it would break compatibility but obviously I find the current behaviour annoying :-)
msg306892 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-24 12:16
While we are here, what if make a negative limit truncating from the opposite side, as in the traceback module?
msg306897 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-24 13:31
Serhiy:
> While we are here, what if make a negative limit truncating from the opposite side, as in the traceback module?

Why not. I'm not opposed to that :-)
msg307272 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-29 23:05
New changeset 706e10b186992e086e661a62d2c8ec9588525b31 by Victor Stinner (Jesse-Bakker) in branch 'master':
bpo-32121: Add most_recent_first parameter to tracemalloc.Traceback.format (#4534)
https://github.com/python/cpython/commit/706e10b186992e086e661a62d2c8ec9588525b31
msg307273 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-29 23:05
Jesse Bakker implemented all requested changes, I merged his PR. Well done, Jesse!
History
Date User Action Args
2017-11-29 23:05:52vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg307273

stage: patch review -> resolved
2017-11-29 23:05:09vstinnersetmessages: + msg307272
2017-11-24 13:31:11vstinnersetmessages: + msg306897
2017-11-24 12:16:26serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg306892
2017-11-24 10:57:28pitrousetmessages: + msg306886
2017-11-24 10:53:00vstinnersetmessages: + msg306885
2017-11-24 00:08:38jbakkersetnosy: + jbakker
messages: + msg306863
2017-11-24 00:06:18jbakkersetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request4470
2017-11-23 21:29:20vstinnersetmessages: + msg306859
2017-11-23 20:10:20pitroucreate