Repository navigation
JSON renderer - #46
Conversation
joerick
left a comment
There was a problem hiding this comment.
Thanks iddan - I think the general output format is the right approach.
Could you add at least one test that reads the output of this renderer and checks there are the expected functions there?
| def _json_frame(frame): | ||
| return { | ||
| 'function': frame.function, | ||
| 'file_path': frame.file_path_short, |
There was a problem hiding this comment.
In the interests of creating a generally useful solution, I'd rather this was called file_path_short, in case we want to add file_path in the future.
|
|
||
| return result | ||
|
|
||
| def _json_frame(frame): |
There was a problem hiding this comment.
Can this be a method on the JSONRenderer object?
| preferred_recorder = 'time_aggregating' | ||
|
|
||
| def render(self, frame): | ||
| return json.dumps(_json_frame(frame)) |
There was a problem hiding this comment.
might be nice to add indent=2 to make the output prettier on the terminal
There was a problem hiding this comment.
I though about it: many outputs can generate big outputs so adding spacing chars would increase their size significantly. Maybe this should be another renderer / CLI option?
If you go the UNIX way you can always do: python -m pyinstrument script.py | python -m json.tool
There was a problem hiding this comment.
Hmm. I don't agree. JSON is a really verbose format. All those repeated key names! All those numbers written out as strings.
The big advantage of JSON is human-readability - you can look at it and quickly understand it. To strip indentation is to lose that.
If size becomes a meaningful issue the right answer would be to remove frames with tiny durations, or use something like Protobuf.
(Re. piping through python -m json.tool - it's a good idea, but I don't think the burden of a good user experience should be on the user)
|
Issued the comments |
|
Here is the app expecting the result: https://python-flame-chart.netlify.com |
|
Just waiting on the |
|
Done! |
|
Cheers @iddan ! |
|
Please let me know when it is released |
|
Apologies for the delay - working on a few more changes and tests before the next release. In the meantime, you can install from the master branch directly - |
|
Released as 2.2.1! |
Issues #44