PERF: speed up concat on Series by skipping unnecessary DataFrame creation

[qwhelan]

Removes an unnecessary DataFrame creation when dealing solely with Series objects, which reduces runtime of concat.

• xref PERF: concat perf #23362
• tests passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

Performance Comparison

import pandas as pd
import numpy as np
a = np.random.randint(2000, 2100, size=100000)  # larger n than #23362 
b = np.random.randint(2000, 2100, size=100000)

x = pd.core.arrays.period_array(a, freq='B')
y = pd.core.arrays.period_array(b, freq='B')

s = pd.Series(x)
t = pd.Series(y)

Baseline

%timeit x._concat_same_type([x, y])
202 µs ± 10.8 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

%timeit pd.concat([s, t], ignore_index=True)
839 µs ± 32.6 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

After

%timeit pd.concat([s, t], ignore_index=True)
396 µs ± 12.5 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

And the excised code itself:

In [13]: pd.DataFrame()._get_axis_number(0)
Out[13]: 0

In [14]: %timeit pd.DataFrame()._get_axis_number(0)
Out[14]: 312 µs ± 23.7 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

So roughly 40% of the runtime was being spent mapping axis=0 -> axis=0

[pep8speaks]

Hello @qwhelan! Thanks for updating the PR.

• There are no PEP8 issues in the file pandas/core/generic.py !

• There are no PEP8 issues in the file pandas/core/reshape/concat.py !

• There are no PEP8 issues in the file pandas/tests/generic/test_generic.py !

Comment last updated on November 02, 2018 at 00:32 Hours UTC

[codecov]

Codecov Report

Merging #23404 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #23404   +/-   ##
=======================================
  Coverage   92.22%   92.22%           
=======================================
  Files         161      161           
  Lines       51191    51191           
=======================================
  Hits        47210    47210           
  Misses       3981     3981

Flag	Coverage Δ
#multiple	90.6% <100%> (ø)	⬆️
#single	42.26% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/core/reshape/concat.py	97.6% <ø> (ø)	⬆️
pandas/core/generic.py	96.81% <100%> (ø)	⬆️

---

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 93aba79...486d3b8. Read the comment docs.

[jreback]

so this actually affects perf? pls show a before and after

[qwhelan]

@jreback I was waiting for asv to finish before posting benchmarks, but I've updated the initial post with some basic benchmarks as that's taking longer than anticipated.

[jreback]

this change just obfuscates things for very very little gain here. This is a constant time operation that we use all over the codebase.

You can try to make this a @staticmethod instead if you really want to get this in.

[TomAugspurger]

@qwhelan doing this just on Py3 also feels hacky. I think everything in _get_axis_number is static, so you can do

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index db10494f0..14edc1d7f 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -358,18 +358,19 @@ class NDFrame(PandasObject, SelectionMixin):
             d.update(kwargs)
             return cls(data, **d)
 
-    def _get_axis_number(self, axis):
-        axis = self._AXIS_ALIASES.get(axis, axis)
+    @classmethod
+    def _get_axis_number(cls, axis):
+        axis = cls._AXIS_ALIASES.get(axis, axis)
         if is_integer(axis):
-            if axis in self._AXIS_NAMES:
+            if axis in cls._AXIS_NAMES:
                 return axis
         else:
             try:
-                return self._AXIS_NUMBERS[axis]
+                return cls._AXIS_NUMBERS[axis]
             except KeyError:
                 pass
         raise ValueError('No axis named {0} for object type {1}'
-                         .format(axis, type(self)))
+                         .format(axis, cls))
 
     def _get_axis_name(self, axis):
         axis = self._AXIS_ALIASES.get(axis, axis)

Will you see if the same change can be made to

• _get_axis_name
• _get_axis
• _get_block_manager_axis

Then, will you ensure that we have basic tests for using all those methods on classes (as well as instances).

[qwhelan]

@jreback Yeah, I'm aware this change is trivial but just using it as an excuse to get my dev setup fully working for some more substantial perf patches that will be coming soon.

@TomAugspurger Thanks, I'll check into it.

[qwhelan]

@TomAugspurger I implemented the @classmethod change for all but _get_axis() (which is used to get an axis instance, so shouldn't be a @classmethod).

There's decent existing test coverage but I added an explicit test of class vs instance invocations for all valid possible calls for good measure.

[TomAugspurger]

Thanks, LGTM, though you have a linting error:
#23404 (comment)

[qwhelan]

@TomAugspurger Thanks, resolved.

[jreback]

small comment, ping on green.

[jreback]

lgtm. ping on green.

[qwhelan]

@jreback All green, thanks!

[TomAugspurger]

In [4]: %timeit x._concat_same_type([x, y])
   ...:
99.8 µs ± 2.25 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

In [5]: %timeit pd.concat([s, t], ignore_index=True, copy=False)
   ...:
   ...:
194 µs ± 3.98 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

so we're down to 2x. Let me know if you're interested in pushing this further @qwhelan (in a separate PR).

[jreback]

thanks @qwhelan

[qwhelan]

@TomAugspurger Yeah, I'm looking at some related issues if you'd like to keep that issue open for now.