asteval
asteval copied to clipboard
Improve performance of make_symbol_table
This is a continuation of #104. The Interpreter is constructed many times when fitting with lmfit and this calls make_symbol_table. This PR improves performance by taking many checks on the valid symbols in math, sympy and builtins out of the function.
Master:
In [2]: %timeit make_symbol_table()
172 µs ± 3.16 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
This PR:
In [59]: %timeit make_symbol_table()
11.3 µs ± 421 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
Note: the (slighty) reduced coverage is in a part that was only modified by autopep and branches where numpy is not installed, but it seems numpy is installed when running coverage. If it is required to fix this, please suggest how to approach this.
Codecov Report
Merging #106 (6c6313f) into master (74bec39) will decrease coverage by
0.13%. The diff coverage is79.16%.
@@ Coverage Diff @@
## master #106 +/- ##
==========================================
- Coverage 94.39% 94.26% -0.14%
==========================================
Files 4 4
Lines 1446 1448 +2
==========================================
Hits 1365 1365
- Misses 81 83 +2
| Impacted Files | Coverage Δ | |
|---|---|---|
| asteval/astutils.py | 87.74% <79.16%> (-1.15%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 74bec39...6c6313f. Read the comment docs.
@eendebakpt thanks, looks very interesting. I am traveling and may not be able to review this sufficiently for a few weeks. I am not ignoring, just may not get too quickly. Feel free to ping me again on this if I'm going too slow. Thanks again.
@eendebakpt Thanks again, this looks like an impressive speedup. I am not actually concerned about the coverage report -- that's informative but does not really signal an error, especially with massive changes to code blocks.
I verified locally on one machine that all asteval tests and all lmfit tests pass with this change.
So, I will very gratefully merge this: thanks, awesome!