CLN: use dispatch_to_series where possible

[jbrockmendel]

A bunch of PRs touching DataFrame ops have gone through recently. This does some follow-up cleanup to unify the way things are done across a few different methods.

[pep8speaks]

Hello @jbrockmendel! Thanks for updating the PR.

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

• In the file pandas/core/ops.py, following are the PEP8 issues :

Line 1646:13: W504 line break after binary operator

Comment last updated on September 07, 2018 at 16:19 Hours UTC

[jreback]

can you check perf. rebase as well.

[jbrockmendel]

can you check perf.

First attempt to check perf turned up a bug in master:

dti = pd.date_range('2016-01-01', periods=10000)
tdi = pd.timedelta_range('1', periods=10000)
tser = pd.Series(tdi)
df = pd.DataFrame({0: dti, 1: tdi})
>>> df.add(tser, axis=0)

Expected (which the PR gets right):

                              0                      1
0 2016-01-01 00:00:00.000000001 0 days 00:00:00.000000
1 2016-01-03 00:00:00.000000001 2 days 00:00:00.000000
2 2016-01-05 00:00:00.000000001 4 days 00:00:00.000000

master raises

ValueError: operands could not be broadcast together with shapes (20000,) (10000,) 

I'll add a test for this.

Non-broken cases, first a many-column case where we expect master to perform well:

df = pd.DataFrame(np.random.randn(100000, 60))
df[10:20] = df[10:20].astype('f4')
df[20:30] = df[20:30].astype('i8')
df[30:40] = df[30:40].astype('i4')
df[40:50] = df[40:50].astype('u8')
df[50:60] = df[50:60].astype('u4')

In [29]: %timeit out = df.add(df[0], axis=0)
100 loops, best of 3: 18.1 ms per loop   <-- master
100 loops, best of 3: 18.3 ms per loop  <-- PR

And a few-column case where we expect master to do poorly:

df = pd.DataFrame(np.random.randn(10000000, 6))
df[1] = df[1].astype('f4')
df[2] = df[2].astype('i8')
df[3] = df[3].astype('i4')
df[4] = df[4].astype('u8')
df[5] = df[5].astype('u4')

%timeit out = df.add(df[0], axis=0)
1 loop, best of 3: 903 ms per loop   <-- master
1 loop, best of 3: 582 ms per loop   <-- PR

As elsewhere, I expect perf to improve after #22284.

[jreback]

this duplicates #22572 a lot. is this the original?

[jbrockmendel]

this duplicates #22572 a lot. is this the original?

This is original. In the course of profiling this I found that this fixes a previously-unknown bug. So this now needs tests and whatsnew etc. #22572 splits off the still-easy part of this.

[jbrockmendel]

To fully finish this off will require resolution to #22614.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@5eb9988). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #22534   +/-   ##
=========================================
  Coverage          ?   92.05%           
=========================================
  Files             ?      169           
  Lines             ?    50787           
  Branches          ?        0           
=========================================
  Hits              ?    46753           
  Misses            ?     4034           
  Partials          ?        0

Flag	Coverage Δ
#multiple	90.46% <100%> (?)
#single	42.29% <0%> (?)

Impacted Files	Coverage Δ
pandas/core/ops.py	96.91% <100%> (ø)
pandas/core/frame.py	97.2% <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 5eb9988...8f0cdbc. Read the comment docs.

[jreback]

can you rebase

[jbrockmendel]

can you rebase

Sure. Big evening for merging. Note comment above about #22614.

[jbrockmendel]

Heh, looks like the other PRs merged this evening already cover this. Closing. Will need to follow-up with a test for the bug that was accidentally fixed.

[jreback]

great! I am not really sure about #22614, none of the options are really palatable.

[jbrockmendel]

I am not really sure about #22614, none of the options are really palatable.

Well we've de-facto been going down the path of option 1. I actually prefer option 2 longer-term (better to discuss there), but for the time being correctness-first seems to favor option 1, and #22284 should take some of the pain out of it.