It's not implemented yet. I did start implementing it locally, but this needs to be coordinated with @iguananaut. Maybe I can already add my connect.py file and then we can make us of to_table/from_table in astropy.io.fits once that's implemented?

Converting an Astropy table to FITS shouldn't be that hard at the moment. But it needs to be coordinated within the larger effort of supporting the new table interface in pyfits/astropy and getting away from the old, partially broken table interface. See #199.

Representing any arbitrary FITS table as an Astropy table will require a fair bit of extra work, and probably a specialized subclass of Table (FITSTable?)

@iguananaut - unless you have any objections, I'd like to open a pull request for an initial implementation of a reader/writer for FITS for astropy.table. At the moment, something 90% functional would be better than nothing, and we can easily improve the integration once pyfits/astropy.io.fits moves to using the Table class internally.

+1 @astrofrog - I think that getting the 90% use case isn't terribly difficult and would be a big win.

@iguananaut - can FITS header keywords repeat or are they required to be unique? I was wondering if the standard model for meta keywords should be an ordered dict or if it should be list-like to allow for repeats. Even in the list-like case there would be keyword access, but if there were repeats then it would return a list of matching keyword objects.

Yes, +1 to that.

And yes, FITS keywords can be duplicates. I would suggest that using the Header object itself for the metadata object. It's already fully duck-typeable as a dict. I wouldn't recommend allowing repeat metadata keywords in the general case, but for FITS and FITS alone it's a requirement.

I'm snowed under this week, but I've attached the skeleton code I have so far.

@iguananaut - how do you actually create a repeat keyword programmatically with PyFITS/astropy.io.fits?

Well, there are two types of repeat keywords in FITS header. The first kind are commentary keywords ('COMMENT', "HISTORY', '') which can be freely repeated. Any time you do header['HISTORY'] = '...' it adds a new HISTORY keyword rather than overriding the value of an existing one.

Any other repeated keyword is technically allowed, but discouraged strongly enough that pyfits doesn't make it easy to do. The only way I can think of is in the constructor: h = pyfits.Header([('ASDF', 'A'), ('ASDF', 'B')])

@iguananaut - just out of curiosity, how do you access the second ASDF keyword, since h['ASDF'] only returns the first?

I don't think that Table.meta should be a Header instance as things might not work trivially when reading/writing to other formats. I'll make an update to this PR to show what I was thinking.

I've added preliminary reading/writing of meta-data. Comments, history, and repeated keywords get stored as lists, and then written back out correctly again. The reason we can't store the metadata as a header object is because we'd have to make sure we always update it if we e.g. move columns around. With the code in this PR, I plan to add that the column-specific meta-data gets assigned to columns instead, so then they get updated if columns get moved around, added, or removed.

This is now in a much better state. The only things that need to be done before this is merged IMHO:

• deal with masked FITS tables
• write tests
• decide whether not specifying should emit a warning or raise an exception

For the last point, it's worth noting that right now the votable reader/write will raise an exception and list the available tables. I've changed it to a warning here for now because FITS tables are a bit simpler, so it's easier to figure out which HDU is needed (whereas in VO tables, that's not the case). What do others thing should be done here? More verbose exception, or just a warning that the first table found was used?

@iguananaut - is there anything else that you think needs to be done before this can be merged?

By the way, I know there was a discussion about overhauling how the registry works to use classes instead of free-floating functions, but I think this can be dealt with separately, and most of the code here can be re-used.

I ran into an issue when implementing the masking - the null values don't appear to be written even if set in the ColDefs (in astropy.io.fits). See #996.

I've now added proper support for masking (now tht #996 has been merged) and also have added units I/O

@taldcroft @iguananaut - could you give this a review and let me know if I'm missing anything obvious?

@iguananaut - I've fixed the issues with the exception of the empty values. Things aren't round-tripping in Python 3 (bytes vs unicode) so I need to understand whether that is a limitation of astropy.io.fits, Numpy, or whether it's an issue with my code (if you have any insight into this, let me know!)

@iguananaut - ah I think I understand - so in astropy.io.fits, the Numpy structured arrays will contain bytes, not unicode, but then when accessing an individual column it will re-cast to unicode? However, when initializing Table from the data array, we don't benefit from this 'magic' so the columns remain in bytes. I wonder what the best solution is - iterate over the columns and add them one by one? Or is there a better way to do this and benefit from the auto-conversion?

Ah, I was more caught up in looking how headers are dealt with that I didn't even look much at how you were copying the table data. Now I see that it's just handled automatically by Table._init_from_ndarray. Though that should be doing the right thing? It looks like in that function it loops through the column names and generates a list of column arrays. From https://github.com/astropy/astropy/blob/master/astropy/table/table.py#L1148:

        data_names = data.dtype.names or _auto_names(n_cols)
        struct = data.dtype.names is not None
        names = [name or data_names[i] for i, name in enumerate(names)]

        cols = ([data[name] for name in data_names] if struct else
                [data[:, i] for i in range(n_cols)])

With the FITS_rec object returned by BinTableHDU.data, doing data[colname] should invoke the "magic" that returns the column converted to the correct data type.

This doesn't just apply to string columns--there are several kinds of columns that need to go through a conversion process, which is what makes FITS tables somewhat complicated to deal with. For example, 'L' columns have to go through the process of converting the 'T' and 'F' strings to True and False boolean values. And don't even get me started on TSCAL and TZERO :)

I managed to fix the issue with string vs unicode in the end, it was an issue on my side.

Ok, I think we are getting there. There is now one remaining issue which is that there are apparently opened files that aren't getting closed. If I run the tests in Python 3 with --open-files I get two failed tests, while Python 2 will give one. However, I can't track down where the file is not getting closed - @iguananaut do you have any ideas?

Wait, I think I know...

@mdboom @taldcroft - I'm running into an interesting (to say the least) bug related to setting units in tables. If I run the tests in Python 3 with the --open-files option, I get:

E           AssertionError: File(s) not closed:
E             /private/var/folders/z4/y7wn3c9x505gg6x3wfjflm4r0000gn/T/pytest-31/test_with_units0/test_with_units.fits

for the test_with_units test. Now this seems to be entirely due to me setting units in the table. If I comment out the code to set and test the units, the test passes. Told you it was interesting! Any ideas at all on what might be going on?

I've rebased to force Travis to run.

The build passed on Travis, so if @iguananaut gives the go-ahead, then I'll merge this.

Fine by me--you're the boss.

@iguananaut I didn't mean to be pushy - sorry! Just want to make sure we have something basic in there already and then focus on refactoring the whole Table I/O next if we can.

@eteq - do you want to review this before I merge?

@iguananaut @taldcroft - I've opened #1046 so we can discuss some of the unresolved issues/questions. Feel free to add to them if there is anything I missed.

Looks fine to me!