issue_comments: 184041128

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/731#issuecomment-184041128 https://api.github.com/repos/pydata/xarray/issues/731 184041128 MDEyOklzc3VlQ29tbWVudDE4NDA0MTEyOA== 1217238 2016-02-15T03:01:22Z 2016-02-15T03:01:57Z MEMBER

I'm pleased to see how this is coming along. No need to apologize for the delay -- this stuff takes time and practice to figure out :).

I'd like to see more test cases so we can be confident this works properly. At the very least, we should test a dot product with multiple dimensions in common. Other edge cases worth checking: - all dimensions in common (e.g., dot product of an array with itself) - no dimensions in common (this should give a sensible error message -- does it?) - dot product with a scalar DataArray (should also error) - dot product with a Dataset (verify that this raises NotImplementedError using self.assertRaises or self.assertRaisesRegexp) - dot product with a numpy array (this should also error)

In generally, it's hard to have too many tests. One indication that you may not have enough tests comes from the "coveralls" status check, which you can find if you click on "Show all checks" at the bottom of the PR. Ideally, each PR should only increase, not decrease code coverage -- the idea is that unit tests should run over every possible code pathway.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   129286220