ENH: Add matmul to DataFrame, Series #19035

bnaul · 2018-01-02T18:06:28Z

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.

codecov · 2018-01-02T19:19:28Z

Codecov Report

Merging #19035 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            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%)`	⬇️
#single	`41.9% <50%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/series.py	`93.87% <100%> (+0.01%)`	⬆️
pandas/core/frame.py	`97.19% <100%> (ø)`	⬆️
pandas/plotting/_converter.py	`65.07% <0%> (-1.74%)`	⬇️
pandas/core/generic.py	`95.85% <0%> (ø)`	⬆️
pandas/util/_decorators.py	`82.4% <0%> (+0.14%)`	⬆️
pandas/util/testing.py	`84.73% <0%> (+0.2%)`	⬆️
pandas/core/nanops.py	`96.69% <0%> (+0.39%)`	⬆️

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 0a00365...c036ed0. Read the comment docs.

gfyoung · 2018-01-02T22:13:00Z

pandas/tests/frame/test_analytics.py

@@ -2067,6 +2067,58 @@ def test_dot(self):
        with tm.assert_raises_regex(ValueError, 'aligned'):
            df.dot(df2)

+    def test_matmul(self):


Reference issue number.

gfyoung · 2018-01-02T22:14:08Z

pandas/core/series.py

@@ -1625,6 +1625,13 @@ def dot(self, other):
        else:  # pragma: no cover
            raise TypeError('unsupported type: %s' % type(other))

+    def __matmul__(self, other):
+        from pandas.core.frame import DataFrame
+        dot_types = (Index, DataFrame, Series, np.ndarray, list)


Your tests need to ensure that for every supported dot type, this operation works. I don't think that's the case currently in your changes.

gfyoung · 2018-01-02T22:14:48Z

pandas/core/frame.py

@@ -866,6 +866,12 @@ def dot(self, other):
        else:  # pragma: no cover
            raise TypeError('unsupported type: %s' % type(other))

+    def __matmul__(self, other):
+        dot_types = (Index, DataFrame, Series, np.ndarray, list)


I feel like this should become a common resource, since you use it in multiple places.

jreback

pls add a whatsnew note in other enhancements

jreback · 2018-01-03T11:36:33Z

pandas/core/frame.py

+        dot_types = (Index, DataFrame, Series, np.ndarray, list)
+        if not isinstance(other, dot_types):
+            return NotImplemented
+        return self.dot(other)


you don't need to specify the dot_types here, these are already handled in .dot (though maybe not tested fully). if something is rejected it should be there

jreback · 2018-01-03T11:37:26Z

pandas/tests/frame/test_analytics.py

@@ -2067,6 +2067,58 @@ def test_dot(self):
        with tm.assert_raises_regex(ValueError, 'aligned'):
            df.dot(df2)

+    def test_matmul(self):
+        a = DataFrame(np.random.randn(3, 4), index=['a', 'b', 'c'],


instead of writing a new test, just parameterize test_dot on dot/matmul as this is pretty much the same

you will need to skip the matmul tests if not PY35

jreback · 2018-01-03T11:37:38Z

pandas/tests/series/test_analytics.py

@@ -837,6 +837,32 @@ def test_dot(self):
        pytest.raises(Exception, a.dot, a.values[:3])
        pytest.raises(ValueError, a.dot, b.T)

+    def test_matmul(self):


bnaul · 2018-01-06T23:15:07Z

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 #10259 (comment)).

Re: Python<3.5: it seems like we can still run the test for .__matmul__ even though the @ operator doesn't exist, right?

jreback · 2018-01-06T23:40:31Z

pandas/core/series.py

@@ -1596,6 +1596,12 @@ def dot(self, other):
        else:  # pragma: no cover
            raise TypeError('unsupported type: %s' % type(other))

+    def __matmul__(self, other):
+        try:


can u add a docstring

jreback · 2018-01-06T23:40:46Z

pandas/core/frame.py

@@ -874,6 +874,12 @@ def dot(self, other):
        else:  # pragma: no cover
            raise TypeError('unsupported type: %s' % type(other))

+    def __matmul__(self, other):
+        try:


jreback · 2018-01-06T23:41:10Z

pandas/tests/frame/test_analytics.py

@@ -2075,41 +2075,42 @@ def test_clip_with_na_args(self):
                              self.frame)

    # Matrix-like
-
+    @pytest.mark.parametrize('dot_fn', [DataFrame.dot, DataFrame.__matmul__])


can u add a test that hits the not implemented error

jreback · 2018-01-06T23:41:44Z

doc/source/whatsnew/v0.23.0.txt

@@ -202,6 +202,7 @@ Other Enhancements
 - ``Resampler`` objects now have a functioning :attr:`~pandas.core.resample.Resampler.pipe` method.
  Previously, calls to ``pipe`` were diverted to  the ``mean`` method (:issue:`17905`).
 - :func:`~pandas.api.types.is_scalar` now returns ``True`` for ``DateOffset`` objects (:issue:`18943`).
+- :class:`DataFrame` and :class:`Series` now support matrix multiplication (```@```) operator (:issue:`10259`)


say for >= 3.5

jreback · 2018-01-06T23:42:47Z

pandas/core/series.py

@@ -1596,6 +1596,12 @@ def dot(self, other):
        else:  # pragma: no cover
            raise TypeError('unsupported type: %s' % type(other))

+    def __matmul__(self, other):


can u update the .dot doc string that ‘@‘ operator is supported in >=3.5

can you do this

You mean update the .dot doc string? I believe that is done

jreback · 2018-02-10T18:45:07Z

can you rebase / update

bnaul · 2018-02-19T20:08:22Z

@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.

jreback

small comments. lgtm otherwise. yeah I think the np.asarray(other) call is actually ok.

jreback · 2018-02-19T20:12:50Z

pandas/core/frame.py

+    def __matmul__(self, other):
+        """ Matrix multiplication using binary `@` operator in Python>=3.5 """
+        try:
+            return self.dot(other)


so let's remove the try/except then

jreback · 2018-02-19T20:12:54Z

pandas/core/series.py

+    def __matmul__(self, other):
+        """ Matrix multiplication using binary `@` operator in Python>=3.5 """
+        try:
+            return self.dot(other)


bnaul · 2018-02-20T01:45:02Z

removed!

jreback · 2018-02-20T01:51:17Z

lgtm. ping on green!

bnaul · 2018-02-20T04:07:34Z

@jreback 💚

shoyer · 2018-02-20T07:03:57Z

pandas/tests/series/test_analytics.py

@@ -895,28 +895,30 @@ def test_count(self):
        ts.iloc[[0, 3, 5]] = nan
        assert_series_equal(ts.count(level=1), right - 1)

-    def test_dot(self):
+    @pytest.mark.parametrize('dot_fn', [Series.dot, Series.__matmul__])


Prefer operator.matmul (which corresponds to the @ operator) to Series.__matmul__.

shoyer · 2018-02-20T07:11:08Z

You really should implement __rmatmul__ here, so series @ ndarray works as well as ndarray @ series. This will be pretty surprising otherwise.

shoyer · 2018-02-20T07:14:21Z

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.

jreback · 2018-03-16T22:13:10Z

can you rebase and update

pep8speaks · 2018-03-17T19:38:26Z

Hello @bnaul! Thanks for updating the PR.

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

Comment last updated on March 27, 2018 at 23:15 Hours UTC

bnaul · 2018-03-17T19:51:39Z

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.

jreback · 2018-03-25T23:02:16Z

@bnaul can you rebase

jreback · 2018-03-25T23:01:20Z

pandas/core/series.py

@@ -1596,6 +1596,12 @@ def dot(self, other):
        else:  # pragma: no cover
            raise TypeError('unsupported type: %s' % type(other))

+    def __matmul__(self, other):


can you do this

jreback · 2018-03-25T23:02:51Z

@jbrockmendel do these belong in ops.py rather as defined in-line?

jbrockmendel · 2018-03-25T23:12:50Z

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.

bnaul · 2018-03-27T23:59:33Z

@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.

jbrockmendel · 2018-03-28T02:45:25Z

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

bnaul · 2018-03-28T17:13:30Z

Makes sense, lmk if you think the tests I added missed anything

@jreback I think I already made the change you requested

jbrockmendel · 2018-03-29T03:01:44Z

Looks good, thanks

jreback · 2018-03-30T21:57:22Z

thanks @bnaul nice patch!

gfyoung added Compat pandas objects compatability with Numpy or Python functions Numeric Operations Arithmetic, Comparison, and Logical operations Python 3.5 labels Jan 2, 2018

gfyoung reviewed Jan 2, 2018

View reviewed changes

jreback requested changes Jan 3, 2018

View reviewed changes

bnaul force-pushed the matmul branch 2 times, most recently from 5519e13 to d5110ed Compare January 6, 2018 23:12

jreback requested changes Jan 6, 2018

View reviewed changes

bnaul force-pushed the matmul branch 2 times, most recently from 36a0cb8 to 419c9d2 Compare February 19, 2018 20:02

jreback requested changes Feb 19, 2018

View reviewed changes

bnaul force-pushed the matmul branch from 419c9d2 to f545857 Compare February 20, 2018 01:42

jreback added this to the 0.23.0 milestone Feb 20, 2018

jreback approved these changes Feb 20, 2018

View reviewed changes

shoyer reviewed Feb 20, 2018

View reviewed changes

bnaul force-pushed the matmul branch 2 times, most recently from 9922a78 to 41bc7b1 Compare March 17, 2018 19:38

bnaul force-pushed the matmul branch 2 times, most recently from e391293 to 236bae4 Compare March 17, 2018 19:47

jreback requested changes Mar 25, 2018

View reviewed changes

bnaul added 2 commits March 27, 2018 16:15

Add matmul to DataFrame, Series

7db1e25

Add rmatmul to DataFrame, Series

d6f156b

bnaul force-pushed the matmul branch from 236bae4 to d6f156b Compare March 27, 2018 23:15

Mixed dtype matmul tests

396a456

jreback added 2 commits March 30, 2018 17:49

Merge branch 'master' into PR_TOOL_MERGE_PR_19035

ff0b2e1

use compat for version check

c036ed0

jreback approved these changes Mar 30, 2018

View reviewed changes

jreback merged commit d7a4f5b into pandas-dev:master Mar 30, 2018

kornilova203 pushed a commit to kornilova203/pandas that referenced this pull request Apr 23, 2018

ENH: Add matmul to DataFrame, Series (pandas-dev#19035)

94662fa

adbull mentioned this pull request Jun 18, 2018

BUG: np.array @ pd.Series swaps arguments #21530

Closed

ENH: Add matmul to DataFrame, Series #19035

ENH: Add matmul to DataFrame, Series #19035

Conversation

bnaul commented Jan 2, 2018

codecov bot commented Jan 2, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

gfyoung Jan 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnaul commented Jan 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Feb 10, 2018

bnaul commented Feb 19, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnaul commented Feb 20, 2018

jreback commented Feb 20, 2018

bnaul commented Feb 20, 2018

Choose a reason for hiding this comment

shoyer commented Feb 20, 2018

shoyer commented Feb 20, 2018

jreback commented Mar 16, 2018

pep8speaks commented Mar 17, 2018 • edited Loading

Comment last updated on March 27, 2018 at 23:15 Hours UTC

bnaul commented Mar 17, 2018

jreback commented Mar 25, 2018

Choose a reason for hiding this comment

jreback commented Mar 25, 2018

jbrockmendel commented Mar 25, 2018

bnaul commented Mar 27, 2018

jbrockmendel commented Mar 28, 2018

bnaul commented Mar 28, 2018

jbrockmendel commented Mar 29, 2018

jreback commented Mar 30, 2018

codecov bot commented Jan 2, 2018 •

edited

Loading

gfyoung Jan 2, 2018 •

edited

Loading

bnaul commented Jan 6, 2018 •

edited

Loading

pep8speaks commented Mar 17, 2018 •

edited

Loading