Table column assignment doesn't update column description - #2630

@cdeil - does this extend to other meta-data attributes, such as unit?

does this extend to other meta-data attributes, such as unit?

Yes ... same issue for unit ... continuing the above example:

>>> table['a'] = Column([1, 2, 3], description='original', unit='m')
>>> table['a']
<Column name='a' unit=None format=None description='original'>

Yes, looks like a bug to me.

I've assigned this to @luzuku

I've confirmed that this bug is still present with Astropy master (2a96500).

@luzuku was assigned in 2014. @astrofrog - Please remove that.

I want to help get this fixed, but I've never looked at the Table / Column codebase.
Would it help if I attach a failing regression test here as a starting point?

Fixing this bug is obviously the right thing, but there is now a workaround:

table.replace_column('a', Column([4, 5, 6], description='updated'))

@cdeil - I probably don't have time to drive this home, but I think this works:

diff --git a/astropy/table/table.py b/astropy/table/table.py
index 2577413..e283de5 100644
--- a/astropy/table/table.py
+++ b/astropy/table/table.py
@@ -1249,8 +1249,17 @@ class Table(object):
             n_cols = len(self.columns)
 
             if isinstance(item, six.string_types):
-                # Set an existing column
-                self.columns[item][:] = value
+                # Set *in-place* an existing column
+                col = self.columns[item]
+                col[:] = value
+
+                # Transfer attributes when setting to a Column instance.
+                # TODO later: generalize to mixin columns on the right side (value)
+                if isinstance(value, Column):
+                    for attr in ('description', 'unit', 'format', 'meta'):
+                        setattr(col.info, attr, getattr(value, attr))
+                        # Note that setattr(col.info, ...) is used instead of setattr(col, ..)
+                        # because this generalizes to mixins (on the left side).
 

@taldcroft - Thanks!

As a first step, I'll attach a regression test and the fix you propose to this issue.

I've attached a commit here that applies the patch posted by @taldcroft above, and adds a regression test.

I locally confirmed that all the asserts in the test fail before applying the patch.

python setup.py test -V -t astropy/table/tests/test_table.py -a '-k test_table_setitem_column'

@taldcroft - Is it useful to put in this fix into 1.3 in it's current form? The TODO comment you left suggests this is not a full fix, and mixin columns will still fail?

@cdeil - now I'm starting to consider whether it would be possible / good to reverse my earlier opinion and make t['a'] = whatever just do replace_column for that. When I look back on my earlier objections in #3809 they don't seem quite so compelling compared to the problems from in-place replacement. Hmm..

Part of the driver is thinking about mixin columns and realizing that really handling that case assuming inplace replacement is just a bit ugly.

@taldcroft - I agree, and if people really want in-place replacement of values for performance, they can do t['a'][:] = ...?

@taldcroft @astrofrog - I don't have an opinion here what should happen exactly, except that it would be nice to get the bug / pitfall fixed.

Let me know if I can be of any help, or just make a new PR or push commits here.

This issue was fixed by @taldcroft in #5556 .
I'm closing it now.

Just leaving a comment here with some explanations, in case I or someone else comes across this issue in the future ...

With master (and soon v1.3), the example above now works as expected, the bug is fixed:

>>> from astropy.table import Table, Column
>>> table = Table()
>>> table['a'] = Column([1, 2, 3], unit='m', description='original')
>>> table['a'] = Column([4, 5, 6], unit='s', description='updated')
>>> table
<Table length=3>
  a  
  s  
int64
-----
    4
    5
    6

The new potential gotcha (or at least changed behaviour) is that the following used to do an in-place update of the values and keep other column attributes like unit or description, but now it will create a new column, and since no extra attributes are given, they will be "lost":

>>> table['a'] = [7, 8, 9]
>>> table['a']
<Column name='a' dtype='int64' length=3>
7
8
9

@taldcroft explained this in #5556 in the changelog entry and the docs.