-
Notifications
You must be signed in to change notification settings - Fork 130
ENH: LineStats aggregation
#380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
line_profiler/__init__.py
Now also importing `line_profiler.line_profiler.LineStats`
line_profiler/line_profiler.py[i]
pathlib
Removed unused import in stub file
LineStats
New `line_profiler._line_profiler.LineStats` subclass helping
with utilization and aggregation, with the following added
methods:
- `__repr__()`
- `__add__()`, `__iadd__()`
- `print()`
- `from_files()`, `to_file()`
- `from_stats_objects()`
LineProfiler
get_stats()
Now returning a `line_profiler.line_profiler.LineStats`
object instead of a `line_profiler._line_profiler.LineStats`
dump_stats(), print_stats()
Now deferring to the respective `LineStats` methods
load_stats()
Now an alias to `LineStats.from_files()`
main()
Updated to be able to take multiple positional arguments
tests/test_cli.py::test_multiple_lprof_files()
Sped up by avoiding spawning new Python processes
|
@Marxlp, please indicate if you would like to have co-authorship on this PR; and if so, also the appropriate email address so we can update the commit messages, and that GitHub can correctly attribute the contribution also to you. Cheers! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #380 +/- ##
==========================================
+ Coverage 87.56% 88.13% +0.56%
==========================================
Files 18 20 +2
Lines 1641 1921 +280
Branches 348 412 +64
==========================================
+ Hits 1437 1693 +256
- Misses 149 168 +19
- Partials 55 60 +5
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| * ``kernprof`` and ``python -m line_profiler`` CLI options | ||
| * ``GlobalProfiler`` configurations, and | ||
| * profiler output (e.g. ``LineProfiler.print_stats()``) formatting | ||
| * ENH: Added capability to combine profiling data both programmatically (``LineStats.__add__()``) and via the CLI (``python -m line_profiler``) (#380, originally proposed in #219) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may belong in a new area for 5.1.0 (not 5.0.1 because enhancements are minor version bumps according to SemVer)!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when that happens, bump the version in the 3 required places (in 6.x I want to consolidate this into a single place: the __init__.py file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR I didn't feel comfortable presuming to bump the minor version – since I was under the assumption that there will be a 5.0.1 before that. But of course it makes better sense as far as semver goes, and if you already intended to do a minor bump I'm more than happy to oblige. The question now I guess will be whether we bump here and now, or stash this away in a dev/5.1.0 branch somewhere (which we don't currently have) until that happens.
Apropos, in principle the unification of the versions can already happen now without a major-version bump, since the days where kernprof could function independently of the line_profiler has been long gone. We can probably just define __version__ once somewhere and import it elsewhere for now, and just remove it where not explicitly needed in 6.0...
Erotemic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Glad you took numeric stability into account.
|
Cheers! Thanks for the review. |
* Stat aggregation
line_profiler/__init__.py
Now also importing `line_profiler.line_profiler.LineStats`
line_profiler/line_profiler.py[i]
pathlib
Removed unused import in stub file
LineStats
New `line_profiler._line_profiler.LineStats` subclass helping
with utilization and aggregation, with the following added
methods:
- `__repr__()`
- `__add__()`, `__iadd__()`
- `print()`
- `from_files()`, `to_file()`
- `from_stats_objects()`
LineProfiler
get_stats()
Now returning a `line_profiler.line_profiler.LineStats`
object instead of a `line_profiler._line_profiler.LineStats`
dump_stats(), print_stats()
Now deferring to the respective `LineStats` methods
load_stats()
Now an alias to `LineStats.from_files()`
main()
Updated to be able to take multiple positional arguments
* Tests
tests/test_cli.py::test_multiple_lprof_files()
New test for using `python -m line_profiler` with multiple `.lprof`
files
tests/test_line_profiler.py::test_load_stats_files()
New test for loading stats files (old and new, single and multiple)
* Speed up test
tests/test_cli.py::test_multiple_lprof_files()
Sped up by avoiding spawning new Python processes
* CHANGELOG entry
* Fix lint
* Add %%lprun_all for easier profiling in IPython * Add additional IPython tests * Add -z option to lprun for compatibility * Ensure the temporary file is always cleaned up * Fix linting error * ENH: `LineStats` aggregation (#380) * Stat aggregation line_profiler/__init__.py Now also importing `line_profiler.line_profiler.LineStats` line_profiler/line_profiler.py[i] pathlib Removed unused import in stub file LineStats New `line_profiler._line_profiler.LineStats` subclass helping with utilization and aggregation, with the following added methods: - `__repr__()` - `__add__()`, `__iadd__()` - `print()` - `from_files()`, `to_file()` - `from_stats_objects()` LineProfiler get_stats() Now returning a `line_profiler.line_profiler.LineStats` object instead of a `line_profiler._line_profiler.LineStats` dump_stats(), print_stats() Now deferring to the respective `LineStats` methods load_stats() Now an alias to `LineStats.from_files()` main() Updated to be able to take multiple positional arguments * Tests tests/test_cli.py::test_multiple_lprof_files() New test for using `python -m line_profiler` with multiple `.lprof` files tests/test_line_profiler.py::test_load_stats_files() New test for loading stats files (old and new, single and multiple) * Speed up test tests/test_cli.py::test_multiple_lprof_files() Sped up by avoiding spawning new Python processes * CHANGELOG entry * Fix lint * Address review comments and add test * Potentially fix Windows string parsing? * `LineProfilerMagics` refactoring, coverage boost, other fixes (#6) * Stub * Consolidate common functionalities - `line_profiler/ipython_extension.py[i]` - General: Added type comments for sanity checks - `_ParseParamResult`: New private class summarizing the result of argument-parsing - `_RunAndProfileResult`: New private class summarizing the result of calling a function with profiling - `_PatchProfilerIntoBuiltins`: New private helper context-manager class for monkey-patching a profiler into `builtins` - `LineProfilerMagics`: - `_parse_parameters()`: - Refactored from `parse_parameters()`, taking over more of the postprocessing boilerplate - Now returning a `_ParseParamResult` - `_run_and_profile()`: New private method to call a callable, profile and time the call, and handle exception-related boilerplate; returns a `_RunAndProfileResult` - `_lprun_get_top_level_profiled_code()`, `_lprun_all_get_rewritten_profiled_code()`: New private helper methods for handling the compilation of code in the `%%lprun_all -p` and `%%lprun_all` cases - `_handle_end()`: New private method to handle end-of-magic boilerplate (IO, return values) - `lprun()`: - Simplified implementation to use the above helper methods - Fixed erroneous error message when a function to be profiled is not found ("Could not find module [...]" -> "Could not find function [...]") - `lprun_all()`: - Simplified implementation to use the above helper methods - Fixed misuse of cell magic in docstring ("%lprun_all [...]" -> "%%lprun_all [...]") - Fixed edge case where the cell is blank - Now ensuring that the profiling output is formatted before deleting tempfiles * More Sphinx-friendly docs * Prettier output and scoping fix - `line_profiler/ipython_extension.py`: - `_RunAndProfileResult`: - `tempfile`: New attribute - `output`: Updated implementation to allow for module- (i.e. top-) level profiling data (from the code object `.tempfile` compiles into) to be formatted correctly - `_make_show_func_wrapper()`: New private helper method for creating a monkey-patched clone of `line_profiler.line_profiler.show_func()` which implements the above - `_PatchProfilerIntoBuiltins`: Simplified implementation to use a `_PatchDict` internally - `_PatchDict`: New private comtext-manager helper class for monkey-patching arbitrary namespaces - `LineProfilerMagics`: - `_run_and_profile()`: Added argument `tempfile`, which is passed to `_RunAndProfileResult` - `_lprun_all_get_rewritten_profiled_code()`: No longer uses `SkipWrapper` - `lprun_all()`: - Removed nesting of the `cell` content into a mock function; the content is now directly written to the tempfile, compiled, and executed, allowing for prettier output - Fixed scoping of names: local names in `cell` are now correctly reflected in `IPython.core.getipython.get_ipython().user_ns` - `tests/test_ipython.py::TestIPython` - `loops`, `lprun_all_cell_body`: Extracted common assets into class attributes - `test_lprun_all_*()`: - Removed line-number offsets induced by the nesting of the cell content into a mock function - Added checks that the cell content is executed in the correct scope * Boost coverage - `line_profiler/ipython_extension.py` - General: Added `# pragma: no cover` to type-checking only stuff - Removed the following unused code: - `SkipWrapper` - `_ParseParamResult.__getitem__()` - `_RunAndProfileResult.return_value` - `tests/test_ipython.py` - `TestIPython`: Refactored into the classes `TestLPRun` and `TestLPRunAll`, which are no longer `unittest.TestCase` subclasses - `TestIPython.test_init()`: Refactored and parametrized into the following tests: - `test_lprun_profiling_targets()`: Test `-m` - `test_lprun_file_io()`: Test `-D` and `-T` - `test_lprun_timer_unit()`: Test `-u` - `test_lprun_skip_zero()`: Test `-s` and `-z` - `test_lprun_exception_handling()`: Test exception handling in the magics - `TestLPRunAll.test_lprun_all_*()`: Updated implementations * Fix grammar in documentation One space after a period is generally considered correct. Also, add a clarification that the -s option is meant to eventually be deprecated, and that -z is used for consistency with the CLI. --------- Co-authored-by: Theelx <[email protected]> * Hotfixes for failing tests on #383 (#7) * Yet another hotfix (#8) --------- Co-authored-by: Terence Tsang <[email protected]> Co-authored-by: Jon Crall <[email protected]>
(Based on @Marxlp's #219 and the changes I proposed there.)
Code changes
line_profiler/line_profiler.py[i]LineStats:New API subclass of
line_profiler/_line_profiler.pyx::LineStatswhich provides the following convenience methods:__add__(),__iadd__(),from_stats_objects():Methods for aggregating the timing data between instances
to_file(),from_files(),print():IO methods
__eq__():Method for comparing instances
LineProfiler.get_stats(),.dump_stats(),.print_stats():Refactored to return/use the new
LineStatsclassmain():Updated to allow for passing more than one
.lproffilesTests changes
tests/test_cli.py::test_multiple_lprof_files():New test checking that
python -m line_profilerhandles multiple.lproffiles correctlytests/test_line_profiler.py::test_load_stats_files():New test checking that
LineStats.from_files()reads one or more.lproffiles correctly, and handles.lprof(pickle) files generated by both new and old versions ofline_profiler