ENH: Add matmul to DataFrame, Series
State: closed

Fixes #10259. Not sure about the exact set of types that should be supported, for now I just copied the classes that are referenced in `dot`.

Labels:
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=h1) Report
> Merging [#19035](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/0a003651a67854b9776c46469a18e20ffd61f141?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/19035/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #19035      +/-   ##
==========================================
- Coverage   91.83%   91.82%   -0.02%     
==========================================
  Files         152      152              
  Lines       49260    49269       +9     
==========================================
+ Hits        45240    45241       +1     
- Misses       4020     4028       +8
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.21% <100%> (-0.02%)` | :arrow_down: |
| #single | `41.9% <50%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.87% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.19% <100%> (ø)` | :arrow_up: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.07% <0%> (-1.74%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `95.85% <0%> (ø)` | :arrow_up: |
| [pandas/util/\_decorators.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX2RlY29yYXRvcnMucHk=) | `82.4% <0%> (+0.14%)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `84.73% <0%> (+0.2%)` | :arrow_up: |
| [pandas/core/nanops.py](https://codecov.io/gh/pandas-dev/pandas/pull/19035/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvbmFub3BzLnB5) | `96.69% <0%> (+0.39%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=footer). Last update [0a00365...c036ed0](https://codecov.io/gh/pandas-dev/pandas/pull/19035?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Per @jreback 's comment I changed the implementation to just run `dot` directly and rely on its type verification; for an unsupported type `dot` will throw `TypeError` and `__matmul__` will catch and return `NotImplemented` (per https://github.com/pandas-dev/pandas/issues/10259#issuecomment-351502252).

Re: Python<3.5: it seems like we can still run the test for `.__matmul__` even though the `@` operator doesn't exist, right?
can you rebase / update
@jreback rebased and added docstrings.

I also tried to add a test for the `NotImplemented` case but I realized that the way `.dot()` is currently structured makes it impossible to actually reach that code: if `other` is not a series or data frame, it gets passed to `rvals = np.asarray(other)`; so the subsequent type-checking will always finish by `elif isinstance(rvals, np.ndarray)` and short-circuit the `TypeError`. This is arguably a slight flaw in `.dot()` but I don't think this particular PR alters the behavior at all.
removed!
lgtm. ping on green!
@jreback 💚 
You really should implement `__rmatmul__` here, so `series @ ndarray` works as well as `ndarray @ series`. This will be pretty surprising otherwise.
> I also tried to add a test for the NotImplemented case but I realized that the way .dot() is currently structured makes it impossible to actually reach that code: if other is not a series or data frame, it gets passed to rvals = np.asarray(other); so the subsequent type-checking will always finish by elif isinstance(rvals, np.ndarray) and short-circuit the TypeError. This is arguably a slight flaw in .dot() but I don't think this particular PR alters the behavior at all.

I don't think this is a great excuse for not implementing `NotImplemented` for `__matmul__`. There's no need for a protocol for `.dot()` since the implementation has been explicitly indicated.

That said, it seems that pandas doesn't currently defer to other objects for any arithmetic operations, so I don't see any particularly good reasons to start here first.
can you rebase and update
Hello @bnaul! Thanks for updating the PR.









Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on March 27, 2018 at 23:15 Hours UTC
Added `__rmatmul__` as @shoyer suggested. Because the functionality is sufficiently different from `.dot` now I separated the two tests.

`Series.__rmatmul__` seems fairly uncomplicated but `DataFrame` is a bit more so. If anyone has thoughts on a better way to implement it I'd be keen to hear it.
@bnaul can you rebase
@jbrockmendel do these belong in ``ops.py`` rather as defined in-line?
> do these belong in ops.py rather as defined in-line?

I think the PR defines the methods in the right place.  Defining things directly in the class makes for more obvious code than pinning from afar.

The tests might belong in `test_arithmetic`.  That could go either way.

@bnaul can I get you to add a couple of mixed-dtype tests?  e.g. a DataFrame with a mix of integer/float columns.
@jbrockmendel What would a mixed dtype test case test exactly? I can't think of a scenario where everything wouldn't just get coerced to floats.
> What would a mixed dtype test case test exactly? I can't think of a scenario where everything wouldn't just get coerced to floats.

exactly that it does get coerced correctly.  Some methods don't play nicely with mixed-dtypes

Makes sense, lmk if you think the tests I added missed anything

@jreback I think I already made the change you requested
Looks good, thanks
thanks @bnaul nice patch!