CLN: replace lambdas with named functions so they are labeled in asv #24629

qwhelan · 2019-01-04T22:37:20Z

This complements a PR in asv (airspeed-velocity/asv#771), but would be worthwhile on its own.

asv compares benchmarks based on the repr() of the parameters, which causes issues when the parameter is an object that doesn't override the default repr() (such as functions, especially lambda functions):

Benchmarks that have stayed the same:

       before           after         ratio
     [24ab22f7]       [d7cef344]
              n/a          997±3μs      n/a  ctors.SeriesConstructors.time_series_constructor(<function SeriesConstructors.<lambda> at 0x7f0deb0800d0>, False)
              n/a         1.05±0ms      n/a  ctors.SeriesConstructors.time_series_constructor(<function SeriesConstructors.<lambda> at 0x7f0deb0800d0>, True)
      1.04±0.01ms              n/a      n/a  ctors.SeriesConstructors.time_series_constructor(<function SeriesConstructors.<lambda> at 0x7fd9d0bbc598>, False)
      1.06±0.02ms              n/a      n/a  ctors.SeriesConstructors.time_series_constructor(<function SeriesConstructors.<lambda> at 0x7fd9d0bbc598>, True)

As the memory address component of the name changes between runs, asv fails at lining up the comparison. This has to be fixed upstream.

However, passing lambda functions is particularly unhelpful, as we have no idea which of the six such objects are represented in the example above.

This PR simply gives each function a descriptive name:

asv continuous -b SeriesConstructors v0.20.0..HEAD
[...]
[ 50.00%] · For pandas commit 20c2a780 <asv_change> (round 2/2):
[ 50.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 75.00%] ··· ctors.SeriesConstructors.time_series_constructor                                                                                                                                                   ok
[ 75.00%] ··· ======================================================= ============= =============
              --                                                               with_index        
              ------------------------------------------------------- ---------------------------
                                      data_fmt                            False          True    
              ======================================================= ============= =============
                       <function no_change at 0x7efcd7b67c80>           74.0±0.4μs     131±3μs   
                                        list                           1.15±0.03ms   1.20±0.01ms 
                      <function list_of_str at 0x7efcd7b67d08>           688±9μs       741±6μs   
                       <function arr_dict at 0x7efcd7b67b70>           4.49±0.01ms   4.71±0.02ms 
                    <function list_of_tuples at 0x7efcd7b679d8>        1.02±0.01ms   1.10±0.01ms 
                     <function list_of_lists at 0x7efcd7b67a60>        1.04±0.03ms   1.09±0.04ms 
               <function list_of_tuples_with_none at 0x7efcd7b67ae8>   1.02±0.02ms   1.08±0.02ms 
                <function list_of_lists_with_none at 0x7efcd7b676a8>   1.05±0.01ms   1.10±0.01ms 
              ======================================================= ============= =============

[ 75.00%] · For pandas commit 91753873 <maybe_convert_objects_int_overflow_fix~1> (round 2/2):
[ 75.00%] ·· Building for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt.....
[ 75.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[100.00%] ··· ctors.SeriesConstructors.time_series_constructor                                                                                                                                                   ok
[100.00%] ··· ======================================================= ============= =============
              --                                                               with_index        
              ------------------------------------------------------- ---------------------------
                                      data_fmt                            False          True    
              ======================================================= ============= =============
                       <function no_change at 0x7efcd7b67c80>           72.9±0.9μs     126±1μs   
                                        list                             1.10±0ms    1.16±0.02ms 
                      <function list_of_str at 0x7efcd7b67d08>           699±10μs      760±3μs   
                       <function arr_dict at 0x7efcd7b67b70>           4.54±0.08ms   4.71±0.04ms 
                    <function list_of_tuples at 0x7efcd7b679d8>        1.01±0.01ms   1.08±0.02ms 
                     <function list_of_lists at 0x7efcd7b67a60>        1.01±0.01ms   1.07±0.01ms 
               <function list_of_tuples_with_none at 0x7efcd7b67ae8>   1.02±0.01ms   1.09±0.02ms 
                <function list_of_lists_with_none at 0x7efcd7b676a8>     1.05±0ms    1.08±0.01ms 
              ======================================================= ============= =============


BENCHMARKS NOT SIGNIFICANTLY CHANGED.

closes #xxxx
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2019-01-04T23:12:48Z

Codecov Report

Merging #24629 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #24629      +/-   ##
==========================================
- Coverage   92.38%   92.37%   -0.01%     
==========================================
  Files         166      166              
  Lines       52395    52395              
==========================================
- Hits        48403    48402       -1     
- Misses       3992     3993       +1

Flag	Coverage Δ
#multiple	`90.8% <ø> (ø)`	⬆️
#single	`43% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/util/testing.py	`88% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb31b2b...ff15bae. Read the comment docs.

codecov · 2019-01-04T23:12:57Z

Codecov Report

Merging #24629 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #24629   +/-   ##
=======================================
  Coverage   92.38%   92.38%           
=======================================
  Files         166      166           
  Lines       52395    52395           
=======================================
  Hits        48403    48403           
  Misses       3992     3992

Flag	Coverage Δ
#multiple	`90.8% <ø> (ø)`	⬆️
#single	`43% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb31b2b...681518d. Read the comment docs.

WillAyd

generally OK with me. Some real nitpicky comments

WillAyd · 2019-01-04T23:15:41Z

asv_bench/benchmarks/ctors.py

+    return [[i, -i] for i in arr]
+
+
+def list_of_tuples_with_none(arr):


Naming nit but this is a generator expression not a list, no?

Seems the [] forces it into a list, making the outer parens a simple grouping. I'll remove the parens and add generator versions for clarity.

Done - generators don't support +, so didn't add them for this particular case

WillAyd · 2019-01-04T23:15:47Z

asv_bench/benchmarks/ctors.py

+    return ([(i, -i) for i in arr][:-1] + [None])
+
+
+def list_of_lists_with_none(arr):


Same comment

Same as above

WillAyd

LGTM. @jreback not sure if we are merging any non 0.24 items at the moment but think this is OK

jreback · 2019-01-05T14:53:25Z

thanks!

…andas-dev#24629)

mroeschke approved these changes Jan 4, 2019

View reviewed changes

WillAyd reviewed Jan 4, 2019

View reviewed changes

WillAyd added the Benchmark Performance (ASV) benchmarks label Jan 4, 2019

CLN: replace lambdas with named functions so they are labeled in asv

681518d

qwhelan force-pushed the asv_change branch from ff15bae to 681518d Compare January 4, 2019 23:37

WillAyd approved these changes Jan 5, 2019

View reviewed changes

jreback added this to the 0.24.0 milestone Jan 5, 2019

jreback merged commit 994db9f into pandas-dev:master Jan 5, 2019

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CLN: replace lambdas with named functions so they are labeled in asv (p…

f5d2ad9

…andas-dev#24629)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CLN: replace lambdas with named functions so they are labeled in asv (p…

f349d07

…andas-dev#24629)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: replace lambdas with named functions so they are labeled in asv #24629

CLN: replace lambdas with named functions so they are labeled in asv #24629

qwhelan commented Jan 4, 2019

codecov bot commented Jan 4, 2019

codecov bot commented Jan 4, 2019 •

edited

Loading

WillAyd left a comment

WillAyd Jan 4, 2019

qwhelan Jan 4, 2019

qwhelan Jan 4, 2019

WillAyd Jan 4, 2019

qwhelan Jan 4, 2019

WillAyd left a comment

jreback commented Jan 5, 2019

		return [[i, -i] for i in arr]


		def list_of_tuples_with_none(arr):

		return ([(i, -i) for i in arr][:-1] + [None])


		def list_of_lists_with_none(arr):

CLN: replace lambdas with named functions so they are labeled in asv #24629

CLN: replace lambdas with named functions so they are labeled in asv #24629

Conversation

qwhelan commented Jan 4, 2019

codecov bot commented Jan 4, 2019

Codecov Report

codecov bot commented Jan 4, 2019 • edited Loading

Codecov Report

WillAyd left a comment

Choose a reason for hiding this comment

WillAyd Jan 4, 2019

Choose a reason for hiding this comment

qwhelan Jan 4, 2019

Choose a reason for hiding this comment

qwhelan Jan 4, 2019

Choose a reason for hiding this comment

WillAyd Jan 4, 2019

Choose a reason for hiding this comment

qwhelan Jan 4, 2019

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

jreback commented Jan 5, 2019

codecov bot commented Jan 4, 2019 •

edited

Loading