Skip to content

Conversation

@taldcroft
Copy link
Member

I noticed that add_row() was not updated correctly handle the Table mask behavior change in #8789. In particular it was setting the whole Table to be masked if a column became masked, except that the new _set_masked() method is not really doing that job any more.

This is basically a follow-on to #8789 and only affects dev, hence no change log entry required.

The change in data_info.py is a little out of scope, but this is basically fixing an issue where insert in a mixin column would return an object without info.name set correctly (at least for Time, it seems to be OK for Quantity which I did not totally understand). This was being handled by add_row(), but I think it's better to put this into the lower level behavior. I think this issue about insert is so obscure that it does not warrant a change log entry, but if anyone objects I can do that.

OK, it fixes another (never-before-reported and obscure) bug:

In [3]: tm = Time([1], format='cxcsec')
In [4]: t = Table([tm])
In [5]: t.add_row((tm,), mask=(True,))  # this should work!
INFO: Upgrading Table to masked Table. Use Table.filled() to convert to unmasked table. [astropy.table.table]
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
~/miniconda3/lib/python3.6/site-packages/astropy/table/table.py in insert_row(self, index, vals, mask)
   2514                     if self.masked:
-> 2515                         newcol.mask = FalseArray(newcol.shape)
   2516 

AttributeError: can't set attribute

@taldcroft taldcroft added table Affects-dev PRs and issues that do not impact an existing Astropy release no-changelog-entry-needed labels Jun 23, 2019
@taldcroft taldcroft added this to the v4.0 milestone Jun 23, 2019
@taldcroft taldcroft requested a review from mhvk June 23, 2019 16:02
Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@taldcroft taldcroft merged commit af312ae into astropy:master Jun 29, 2019
@taldcroft taldcroft deleted the table-mask-add-row branch June 29, 2019 13:46
@taldcroft
Copy link
Member Author

Thanks @astrofrog ! I actually forgot about this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release no-changelog-entry-needed table

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants