bloomberg / memray

Memray is a memory profiler for Python
https://bloomberg.github.io/memray/
Apache License 2.0
13.17k stars 392 forks source link

Port the tree reporter to textual #499

Closed pablogsal closed 10 months ago

pablogsal commented 10 months ago

Now that we are using Textual for the live mode, we can port the tree reporter to be a live Textual App. This has plenty of advantages over the static version as it offers interactive exploration of the tree, as well as the possibility of using different screens for showing detailed information about allocations such as the source and metadata.

Signed-off-by: Pablo Galindo pablogsal@gmail.com

Issue number of the reported bug or feature request: #

Describe your changes A clear and concise description of the changes you have made.

Testing performed Describe the testing you have performed to ensure that the bug has been addressed, or that the new feature works as planned.

Additional context Add any other context about your contribution here.

godlygeek commented 10 months ago

Some notes:

Overall, I think this is excellent, and a huge improvement over the existing non-interactive tree reporter!

godlygeek commented 10 months ago

Ya know... an alternative to the popup might be to split the window into two panes, both always shown - one would be focusable and let you navigate through the tree, and the other would always display info about the highlighted node. We could even support both - an always-on display for big terminals, and a popup when there's few enough rows/columns that we want to prioritize space efficiency over usability...

pablogsal commented 10 months ago

@willmcgugan do you know if there is an easy way to have a TextArea widget that's frozen and allows highlighting of lines? If not, what would be the best way to get something similar?

willmcgugan commented 10 months ago

@pablogsal Can't think of an straightforward way to do that, but it shouldn't be difficult to add to the TextArea. If you add a feature request to the repo, I think we could get it in the next version.

codecov-commenter commented 10 months ago

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (3f0447b) 92.21% compared to head (793cfe4) 92.30%.

Files Patch % Lines
src/memray/reporters/tree.py 97.14% 5 Missing :warning:
tests/utils.py 71.42% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #499 +/- ## ========================================== + Coverage 92.21% 92.30% +0.09% ========================================== Files 91 91 Lines 11030 11212 +182 Branches 1526 1553 +27 ========================================== + Hits 10171 10349 +178 - Misses 854 858 +4 Partials 5 5 ``` | [Flag](https://app.codecov.io/gh/bloomberg/memray/pull/499/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg) | Coverage Δ | | |---|---|---| | [cpp](https://app.codecov.io/gh/bloomberg/memray/pull/499/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg) | `85.66% <ø> (+0.02%)` | :arrow_up: | | [python_and_cython](https://app.codecov.io/gh/bloomberg/memray/pull/499/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg) | `95.52% <98.37%> (+0.04%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pablogsal commented 10 months ago

small nitpick: can we get a slightly bigger screenshot? It looks like the previous one was about 5000px wide, and the new one is only around 1800px wide. This makes the text harder to read.

Done