Repository: numpy/numpy
Pull request: BUG: fix AttributeError on accessing object in nested MaskedArray.
<!-- Please be sure you are following the instructions in the dev guidelines
http://www.numpy.org/devdocs/dev/development_workflow.html
-->

<!-- We'd appreciate it if your commit message is properly formatted
http://www.numpy.org/devdocs/dev/development_workflow.html#writing-the-commit-message
-->

Closes #15895


Problem statement:
BUG: MaskedArray with nested dtype and object elements cause AttributeError on access
When I create a `MaskedArray` using a nested dtype which contains an `object` at the innermost level, trying to access it causes an _AttributeError: 'str' object has no attribute 'ndim'_.

If I use `int` instead of `object`, the element access works fine.

The issue seems to be caused by the assumption that `_fill_value` is of a numpy type and has an `ndim` attribute.
This assumption is true for `numpy.int32` which would be used if `int` was specified inside the dtype.
But for `object`, the `_fill_value` will be a just python `str` _'?'_, which has no attribute `ndim`.

I propose to change the code line in question to
```python
if hasattr(dout._fill_value, 'ndim') and dout._fill_value.ndim > 0:
```

### Reproducing code example:

```python
import numpy as np
my_dtype = np.dtype([('b', [('c', object)], (1,))])
a = np.ma.masked_all((1,), my_dtype)
print(a['b']['c'])
```

### Error message:

```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python37\lib\site-packages\numpy\ma\core.py", line 3276, in __getitem__
    if dout._fill_value.ndim > 0:
AttributeError: 'str' object has no attribute 'ndim'
```

### Numpy/Python version information:

1.18.1 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 20:34:20) [MSC v.1916 64 bit (AMD64)]

Hints from issue discussion:
Nice catch

but shouldn't we just check if __fill_value is not a ```str``` rather than use hasattr. But even then it feels hacky, there seems to be a comment regarding issue #6723 above the check. 

is this the right approach @seberg ? Trying to understand the background better.
Well, its a bit of a mess. But aside from that, you are right of course. Checking for `ndim` has a bad code smell (and could definitely fail). Even the current code _can_ do something incorrect for certain `object` cases in theory.
I think the solution has to be to test for this happening more directly, one thing I am considering is changing it along:
```
new_fill_value = self._fill_value[indx, ...]  # guarantee array return
```
and moving the `flat` line out of the if. Or, alternative adding an `else: dout._fill_value = new_fill_value[()]`.
So, the purpose of `_fill_value` was to _always_ be an array, unlike `.fill_value` which is the scalar.

Do we know how `_fill_value` is ending up with the wrong type?
Ah sorry, this is of course structured access, which is the problem (so the `...` suggestion will not work on its own). In any case, this code branch seems to not know about `_fill_value` being guaranteed to be an array, and it has to make sure of that.

Probably the best thing to do is to get the new dtype directly with `dtype = self._fill_value.dtype[indx]`, and then using `dtype.ndim` to check for that special branch and otherwise create a new empty array with that dtype and assign to it to ensure that the new `_fill_value` is again an array.
I have looked into the issue, a little more

I have a better understanding of masked array

The solution would be something like this? @seberg 
```python
                    if self._fill_value.dtype[indx].ndim > 0:
                        if not (dout._fill_value ==
                                dout._fill_value.flat[0]).all():
                            warnings.warn(
                                "Upon accessing multidimensional field "
                                f"{indx!s}, need to keep dimensionality "
                                "of fill_value at 0. Discarding "
                                "heterogeneous fill_value and setting "
                                f"all to {dout._fill_value[0]!s}.",
                                stacklevel=2)
                        dout._fill_value = dout._fill_value.flat[0]
                    else:
                        dout._fill_value = array(self._fill_value[indx])
```
Yeah, something along those lines, although I did not check carefully, and there may be some other tricky stuff lurking.

Commit message: Update numpy/ma/tests/test_extras.py

Co-Authored-By: Eric Wieser <wieser.eric@gmail.com>

Patch to review:
diff --git a/numpy/ma/core.py b/numpy/ma/core.py
index fa888107f826..07784b478ab7 100644
--- a/numpy/ma/core.py
+++ b/numpy/ma/core.py
@@ -3305,7 +3305,8 @@ def _scalar_heuristic(arr, elem):
                     # which should have shape () to avoid breaking several
                     # methods. There is no great way out, so set to
                     # first element.  See issue #6723.
-                    if dout._fill_value.ndim > 0:
+                    dtype = self._fill_value.dtype[indx]
+                    if dtype.ndim > 0:
                         if not (dout._fill_value ==
                                 dout._fill_value.flat[0]).all():
                             warnings.warn(
@@ -3316,6 +3317,12 @@ def _scalar_heuristic(arr, elem):
                                 f"all to {dout._fill_value[0]!s}.",
                                 stacklevel=2)
                         dout._fill_value = dout._fill_value.flat[0]
+                    elif not hasattr(dout._fill_value, 'ndim'):
+                        # only initialize to empty array if it dout does not 
+                        # have ._fill_value before. If it has, do not change 
+                        # the original behavior
+                        dout._fill_value = np.empty((), dtype)
+                        dout._fill_value[()] = self._fill_value[indx]
                 dout._isfield = True
             # Update the mask if needed
             if mout is not nomask:
diff --git a/numpy/ma/tests/test_extras.py b/numpy/ma/tests/test_extras.py
index 1c8610625e33..c6300611b1f8 100644
--- a/numpy/ma/tests/test_extras.py
+++ b/numpy/ma/tests/test_extras.py
@@ -64,6 +64,19 @@ def test_masked_all(self):
         control = array([[(1, (1, 1))]], mask=[[(1, (1, 1))]], dtype=dt)
         assert_equal(test, control)
 
+    def test_masked_all_with_dtype_str(self):
+        # Test masked_all works with nested array with dtype of an 'object'
+        # refers to issue #15895
+        my_dtype = np.dtype([('b', ([('c', object)], (1,)))])
+        a = np.ma.masked_all((1,), my_dtype)
+
+        assert_equal(type(a['b']), np.ma.core.MaskedArray)
+        assert_equal(type(a['b']['c']), np.ma.core.MaskedArray)
+        assert_equal(len(a['b']['c']), 1)
+        assert_equal(a['b']['c'].shape, (1, 1))
+        assert_equal(a['b']['c']._fill_value.shape, ())
+``
+
     def test_masked_all_like(self):
         # Tests masked_all
         # Standard dtype


Code review comments:
diff_hunk: @@ -3316,6 +3317,12 @@ def _scalar_heuristic(arr, elem):
                                 f"all to {dout._fill_value[0]!s}.",
                                 stacklevel=2)
                         dout._fill_value = dout._fill_value.flat[0]
+                    elif not hasattr(dout._fill_value, 'ndim'):
+                        # only initialize to empty array if it dout does not 
+                        # have ._fill_value before. If it has, do not change 
+                        # the original behavior
+                        dout._fill_value = np.empty((), dtype)
+                        dout._fill_value[()] = self._fill_value[indx]
path: numpy/ma/core.py
text: @user1:
I suspect that actually the fix is way simpler, and that you can get away with the only change being
```suggestion
                        # need to use squeeze to ensure the result is an array not a scalar
                        dout._fill_value = dout._fill_value.flat[0:1].squeeze(axis=0)
```

Would you mind testing that?

@author:
Oh yes I've tested it and it works! Thank you!

@author:
I'll pushed the tests along with the change

@user1:
Thanks for taking the time to write the tests so that it's easy to try these things!