Fix auto-expansion of string columns when dealing with masked values - #9623

The allowed-to-fail job passed, so all looks OK. And of course both @taldcroft and I should have paid attention to that job in #9559...

Writing tests, it also seemed to make sense that to use the mask of whatever is inserted.

Yikes, that was pretty bad!

I think that the two cases below should give consistent results and that the output dtype should be U3 (str3) in both cases. For MaskedColumn we do support unmasking, so the data underneath any masks should be considered for the auto-expansion:

In [2]: c = MaskedColumn(['a', 'b'])
In [3]: c2 = c.insert(1, ['ccc', 'dd'], mask=[True, False])
In [4]: c2.mask[1] = False
In [5]: c2
Out[5]: 
<MaskedColumn dtype='str3' length=4>
  a
ccc
 dd
  b

In [6]: c2 = c.insert(1, np.ma.MaskedArray(['ccc', 'dd'], mask=[True, False]))
In [8]: c2.mask[1] = False
In [9]: c2
Out[9]: 
<MaskedColumn dtype='str2' length=4>
 a
cc
dd
 b

Yes, that is indeed better; only for np.ma.masked do we really not have any information on what the underlying data could be. It also meant the changes could be simplified beyond what you suggested, making this a very small change.

When inserting masked values, #9559 caused any string column to expand to allow "N/A" to be added.

I think this is good, but I can't figure out how to reproduce this problem on master. Hint please?

@pllim had set the IERS test to xfail just to get master to pass again...

Yes, sorry. You need to remove this line here, which this PR has already done.

In this PR, that line is already removed - so the fact that tests pass is relevant 😄

When inserting masked values, #9559 caused any string column to expand to allow "N/A" to be added.

BTW I finally understand... (slow!). I think the "N/A" was a red-herring for me, but now I see that the original IERS problem is simply because of directly inserting np.ma.masked into a MaskedArray of strings. And of course this is covered exactly in the new test so all good.

No worries - I had exactly the same misunderstanding.

BTW, I'm wondering if this needs a bug fix entry. It actually fixes a real bug in (previous) master unrelated to strings and #9559:

>>> c = MaskedColumn([1, 2])
>>> c.insert(1, np.ma.masked)
<MaskedColumn dtype='int64' length=3>
1
0  <-- Should be -- masked
2

Yes, you're right. Will create a quick PR with a changelog entry.

See #9635