BUG: Implemented MaskedArray.dot - #5709

Otherwise looks good to me.

Be good to get this finished up.

I probably won't get to this until the weekend. I would like to add tests for the out ≠ None case and find a way to avoid calling getmaskarray if a mask is nomask.

Shouldn't this be the same as np.ma.extras.dot? Note that the latter has a strict keyword, but no out. What if out is not an instance of masked array, should that not raise an error explicitly instead of implicitly?

Shouldn't this be the same as np.ma.extras.dot?

No. MaskedArray methods should have the same signature as ndarrays. From the documentation:

The numpy.ma module provides a nearly work-alike replacement for numpy that supports data arrays with masks.

The current sorry state of affairs in np.ma.extras is not the reason to perpetuate the violations of the substitution principle.

Then I'd procede by first adding out to ma.dot and calling it from the method with strict=True. I note that currently the method behavior is different than the strict=False default of the function. Both options for strict could be useful, do you have any thoughts as to how to do that? Or do you think the non-strict version is a bad idea.

I think both strict=False and strict=True are wrong. See my note on the issue.

I am strongly against any differences in ndarray and masked arrays APIs. The original promise of MA was to be a drop-in replacement for numpy. We should try to keep that promise. The strict flag is just developer saying: I have no clue what a dot product of two masked arrays should be, so I'll give two versions hoping that one will do what user wants.

@charris - I would like to start with fixing the bug and then discuss any enhancements. The current situation is that MaskedArray inherits ndarray's dot and appears working with the ndarray.dot's signature, but produces completely wrong results in presence of masked values.

The dot behavior will certainly be revisited when @ operator is implemented and we will probably deprecate dot methods altogether, so there is no reason to try to make it perfect now.

But - it will be a very long time before ".dot" goes out of use - we would have to wait for Python 3.5 to be the oldest version that anyone is likely to use - say 10 years?

@abalkin But the mask returned here looks to propagate the strict=True logic. Am I missing something?

No, in my implementation ma.extra.dot is a call to MaskedArray.dot() after some mask manipulation done if strict=True. So x.dot(y) is the same as ma.dot(x, y, strict=False). This is proven by the fact that ma.dot tests pass unchanged.

You are right, my bad. But why not just call ma.dot with strict=False. After adding an out argument, of course. That way both method and function will be fixed/updated.

Hmm, OK, I missed that ma.dot now called the method. Why not add an out to the function also?

Other fixups can be done later, so merging. @abalkin Could you add a note to 1.10-notes.rst mentioning the change in the dot method behavior? Probably in two places, under Compatibility notes for using the mask, and under New features for the out argument.

@charris - the out argument is not a new feature. The inherited method already had the out argument, it just did not produce correct results. This PR is strictly a bug fix, not a feature.

Point taken. I still think it needs a compatibility mention as the result has changed.

Yes, I'll try to come up with a short compatibility note.

There is a regression here, see #6611. Looking closely at this, I think the code could be made clearer. I also think the rework should be done in the dot function and then used in the method, not the other way round. In any case, if there is no activity to fix this up, I'm going to revert it.

I'll take a look at this tonight as see what can be done to improve the code and fix the regression.

Great, thanks.

@jjhelmus Any progress. This is the last thing that needs fixing before 1.10.2rc1.

Sorry have not had the time to properly look at this. Why don't we revert this for 1.10.2rc1.