Added --aggregate option to attach

[ivonastojanovic]

Added --aggregate option which allows user to request aggregated mode for in-memory aggregation.

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.

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (bb9c0db) 91.88% compared to head (5a2cdef) 91.90%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #455      +/-   ##
==========================================
+ Coverage   91.88%   91.90%   +0.01%     
==========================================
  Files          90       91       +1     
  Lines       10766    10803      +37     
  Branches     1473     1485      +12     
==========================================
+ Hits         9892     9928      +36     
- Misses        872      873       +1     
  Partials        2        2              

Flag	Coverage Δ
cpp	85.01% <ø> (-0.22%)	⬇️
python_and_cython	95.41% <100.00%> (+0.12%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/memray/commands/attach.py	59.17% <100.00%> (+5.23%)	⬆️
tests/integration/test_attach.py	91.22% <100.00%> (+5.11%)	⬆️
tests/unit/test_attach.py	100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[godlygeek]

This parser.error() never fires, because hasattr(args, "output") is always True.

Sometimes the attribute is None, but it always exists. If it didn't, we'd get an AttributeError on this line above:

memray/src/memray/commands/attach.py

Line 332 in 2751eb4

if args.output:

We should add a unit test that exercises this case, too, so that we're not introducing a new, uncovered line of code.

[godlygeek]

Oooh, this is interesting. This winds up passing file_format=1, because:

>>> FileFormat.AGGREGATED_ALLOCATIONS
<FileFormat.AGGREGATED_ALLOCATIONS: 1>
>>> f"file_format={FileFormat.AGGREGATED_ALLOCATIONS}"
'file_format=1'

I'm surprised that works. I think it would be more robust to do:

        file_format = (
            "file_format=memray.FileFormat.AGGREGATED_ALLOCATIONS" if args.aggregate else ""
        )

I'd rather pass around the enum by name than by value, in case a future version of the API changes the value associated with the enum, or changes the Tracker constructor to reject ints and only accept enum members.

I'd like to see an integration test that exercises this, too - that should be as easy as parametrizing our current attach test with a new flag for whether or not to use aggregated mode, passing --aggregate in the attach_cmd when the flag is set, and asserting that get_allocation_records() fails when --aggregate was passed but succeeds when it wasn't, and that get_high_watermark_allocation_records() works in both cases.

[godlygeek]

As we discussed offline I've modified the integration tests a bit, but otherwise this looks great to me! I'm going to squash and land this.