Better error if adding a non-iterable object to a table as a column #3811

Activity

1. astrofrog

   added

   table

   on Jun 2, 2015
2. gregosimo commented on May 25, 2020

   

   gregosimo

   on May 25, 2020

   More actions

   Hello,

   I was interested in contributing to Astropy, and noticed that this in the "good first bug" category. So I was interested in working on this.

   Since its been a while, I re-checked this behavior in the latest development version; the bug behaves slightly differently. Now instead of throwing an exception immediately, the Table throws an exception anytime the data is accessed. For example:

   In [4]: t = Table()
   
   In [5]: t["a"] = 1
   
   In [6]: t
   Out[6]: ---------------------------------------------------------------------------
   AttributeError                            Traceback (most recent call last)
   ~/dev/astropy/astropy/table/table.py in __len__(self)
      1814         try:
   -> 1815             return len(OrderedDict.__getitem__(self.columns, self._first_colname))
      1816         except (AttributeError, KeyError):
   
   AttributeError: 'Table' object has no attribute '_first_colname'
   
   During handling of the above exception, another exception occurred:
   
   TypeError                                 Traceback (most recent call last)
   ~/miniconda3/envs/astropy-dev/lib/python3.7/site-packages/IPython/core/formatters.py in __call__(self, obj)
       700                 type_pprinters=self.type_printers,
       701                 deferred_pprinters=self.deferred_printers)
   --> 702             printer.pretty(obj)
       703             printer.flush()
       704             return stream.getvalue()
   
   ~/miniconda3/envs/astropy-dev/lib/python3.7/site-packages/IPython/lib/pretty.py in pretty(self, obj)
       392                         if cls is not object \
       393                                 and callable(cls.__dict__.get('__repr__')):
   --> 394                             return _repr_pprint(obj, self, cycle)
       395
       396             return _default_pprint(obj, self, cycle)
   
   ~/miniconda3/envs/astropy-dev/lib/python3.7/site-packages/IPython/lib/pretty.py in _repr_pprint(obj, p, cycle)
       682     """A pprint that just redirects to the normal repr function."""
       683     # Find newlines and replace them with p.break_()
   --> 684     output = repr(obj)
       685     lines = output.splitlines()
       686     with p.group():
   
   ~/dev/astropy/astropy/table/table.py in __repr__(self)
      1308
      1309     def __repr__(self):
   -> 1310         return self._base_repr_(html=False, max_width=None)
      1311
      1312     def __str__(self):
   
   ~/dev/astropy/astropy/table/table.py in _base_repr_(self, html, descr_vals, max_width, tableid, show_dtype, max_lines, tableclass)
      1282             if self.masked:
      1283                 descr_vals.append('masked=True')
   -> 1284             descr_vals.append('length={}'.format(len(self)))
      1285
      1286         descr = ' '.join(descr_vals)
   
   ~/dev/astropy/astropy/table/table.py in __len__(self)
      1820             # Get the first column name
      1821             self._first_colname = next(iter(self.columns))
   -> 1822             return len(self.columns[self._first_colname])
      1823
      1824     def index_column(self, name):
   
   TypeError: len() of unsized object
   

   I was thinking it might be useful to go back to having this exception thrown at assignment with a more descriptive name. Perhaps something like:

   >>> t = Table()
   >>> t["a"] = 1
   TypeError: Empty table cannot have column set to scalar value.
   

   It seems like this could be easily done by checking if both the table and column have zero length in the add_column() method and then throwing an exception. Does that seem reasonable?
3. pllim commented on May 26, 2020

   

   pllim

   on May 26, 2020

   Member

   More actions

   Thank you for your interest, @gregosimo !

   this in the "good first bug" category

   It is not labeled as such here, so I am curious where did you see that listed. Doesn't mean you can't work on it though!

   Does that seem reasonable?

   Hard to tell if it has any unintended side effects until it is in PR form and goes through the CI, but I would wait for @taldcroft or @mhvk to advise first.
4. taldcroft commented on May 27, 2020

   

   taldcroft

   on May 27, 2020

   Member

   More actions

   @gregosimo - that would be great if you can have a go at making a PR!
5. gregosimo

   added a commit that references this issue on Jun 8, 2020

   Raise TypeError when scalar value is passed to add_column.

   ...

   This issue will close once commit 451c36e is merged into the 'master' branch.

   451c36e
6. bsipocz

   added a commit that references this issue on Oct 23, 2020

   Raise TypeError when scalar value is passed to add_column.

   ...

   This issue will close once commit ec513d3 is merged into the 'master' branch.

   ec513d3
7. bsipocz

   closed this as completedin #10476on Oct 23, 2020
8. namurphy

   added a commit that references this issue on Feb 21, 2021

   Raise TypeError when scalar value is passed to add_column.

   ...

   This issue will close once commit 382d40d is merged into the 'main' branch.

   382d40d