Better error message for CSV writer in the presence of comments

[pllim]

Fix #7357

Might be painful to backport, especially to v2, so milestoning to 3.1, but please let me know if this needs to be re-milestoned.

[astropy-bot]

Hi there @pllim 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

I noticed the following issue with this pull request:

• Changelog entry section (v3.1) inconsistent with milestone (v3.2)

Would it be possible to fix this? Thanks!

If there are any issues with this message, please report them here.

[bsipocz]

I feel 3.1 is about right, otherwise it would be a small API change for LTS (raising ValueError rather than the current messy TypeError).

[cdeil]

@pllim - Thanks!

Trying out the examples given in #7357 with this branch:

>>> from astropy.table import Table
>>> table = Table.read('https://www.mpi-hd.mpg.de/hfm/HESS/hgps/data/hgps_catalog_v1.fits.gz', hdu=1)
Downloading https://www.mpi-hd.mpg.de/hfm/HESS/hgps/data/hgps_catalog_v1.fits.gz
|=====================================================================================================================================================================|  49k/ 49k (100.00%)         0s
>>> table.write('test.csv', format='ascii.csv', overwrite=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/work/code/astropy/astropy/table/table.py", line 2598, in write
    io_registry.write(self, *args, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/registry.py", line 560, in write
    writer(data, *args, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/connect.py", line 43, in io_write
    return write(table, filename, format=format, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/ui.py", line 912, in write
    writer.write(table, output)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/fastbasic.py", line 214, in write
    raise ValueError(' '.join(errs))
ValueError: Use comment='#' option to enable comments in metadata.
>>> table.write('test.csv', format='ascii.csv', overwrite=True, comment='#')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/work/code/astropy/astropy/table/table.py", line 2598, in write
    io_registry.write(self, *args, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/registry.py", line 560, in write
    writer(data, *args, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/connect.py", line 43, in io_write
    return write(table, filename, format=format, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/ui.py", line 912, in write
    writer.write(table, output)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/fastbasic.py", line 216, in write
    self._write(table, output, {'fill_values': [(core.masked, '')]})
  File "/Users/deil/work/code/astropy/astropy/io/ascii/fastbasic.py", line 179, in _write
    writer.write(output, header_output, output_types)
  File "astropy/io/ascii/cparser.pyx", line 1063, in astropy.io.ascii.cparser.FastWriter.write
    if field in self.fill_values:
TypeError: unhashable type: 'list'

That's another cryptic TypeError. Related or a new issue?

In this case following the suggestion to put comment='#' works:

>>> table = Table(data={'a': [1, 2]}, meta={'comments': ['spam']})
>>> table.write('test.csv', format='ascii.csv', overwrite=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/work/code/astropy/astropy/table/table.py", line 2598, in write
    io_registry.write(self, *args, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/registry.py", line 560, in write
    writer(data, *args, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/connect.py", line 43, in io_write
    return write(table, filename, format=format, **kwargs)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/ui.py", line 912, in write
    writer.write(table, output)
  File "/Users/deil/work/code/astropy/astropy/io/ascii/fastbasic.py", line 214, in write
    raise ValueError(' '.join(errs))
ValueError: Use comment='#' option to enable comments in metadata.
>>> table.write('test.csv', format='ascii.csv', overwrite=True, comment='#')

[astrofrog]

I think a better behavior would be to simply drop the comments with a warning that they can be saved with the suggested workaround. That is, I think it should just work by default and not output comments, and users can opt in to include comments. I don't think it should crash by default.

[pllim]

Ops, the crashing was unexpected. I tested with the simple case and did not try with the *.gz download. I'll see to Tom R's idea.

[pllim]

I do not understand the TypeError: unhashable type: 'list' error as well given by *gz case, as its table comments are list of str as required. It must be something I overlooked. @taldcroft ?

[pllim]

p.s. Table.read does not use the download cache?

[pllim]

Update: I have a feeling that TypeError: unhashable type: 'list' is unrelated to the comments. I copied over the comments from *gz case to simple case, and the simple case still writes fine. I suspect there is something in the table data that fast writer cannot handle, because it writes out fine when fast_writer=False.

[pllim]

@astrofrog , I am not sure if I want to drop the comments, because that requires making a copy of the table. Otherwise, the table object loses its comments just because .write() was called. What if you want to write it out but still manipulate it and re-write it out to a different format afterwards?

[pllim]

Would silently applying comment='#' and writing it out when comments are present be desirable?

[astrofrog]

@astrofrog , I am not sure if I want to drop the comments, because that requires making a copy of the table. Otherwise, the table object loses its comments just because .write() was called. What if you want to write it out but still manipulate it and re-write it out to a different format afterwards?

Just to be clear, I'm only suggesting dropping it at the point of writing - that is, we would just not write them but still preserve them in the table.

[pllim]

The path forward for this PR was unclear to me. Is there more changes I am supposed to do? If this cannot get in for 3.1, feel free to re-milestone.

[bsipocz]

@pllim @astrofrog - what is the way forward for this PR? Shall we include it in 3.2?

[pllim]

From #7801 (comment) , looks like I was confused and still am... 😬

[pllim]

I don't remember what I was doing anymore.