Votable/table interface by astrofrog · Pull Request #524 · astropy/astropy

astrofrog · 2012-12-04T11:29:20Z

This PR adds the ability to read/write VO table directly from the astropy.table.Table class. I still need to add tests. In the mean time, @mdboom, do you see any issues with this approach? This allows users to do:

In [2]: from astropy.table import Table

In [3]: t = Table.read('aj285677t3_votable.xml', format='votable')

In [4]: t.write('test.xml', format='votable')

If there is nothing fundamentally wrong with this, I will start adding some tests.

…class

mdboom · 2012-12-04T16:14:07Z

astropy.io.votable.table.from_table will create the VOTableFile from a Table. I don't think there's a need for the following lines, other than to_xml to write out.

Ah -- I see why you're doing this -- to set the ID of the table -- but it looks like you're setting the ID of the VOTable file, not the table anyway. I think the better thing is to add ID support to astropy.io.votable.table.from_table and handle it there.

Does it make sense to change from_table to:

def from_table(cls, votable, table, id=None, name=None, ref=None, ucd=None, utype=None, description=None)

and then read the values from meta if available and not specified in the above call? Or would it make sense to just do it for ID?

That's an interesting idea. The meta stuff there now is just to make everything round trip when converting to a Table and back... I think it's probably best to just have the user set these things after the fact (since this set of things may change over time). But I'll think on it -- my view may change. In any case, that probably doesn't matter for this PR.

Ok - so for the ID stuff, shall I add the id argument to from_table for now since I think that is what you were suggesting before? (unless I misunderstood). Or should we just require the user to set t.meta['ID'] before writing to a file if they want to set a custom ID?

I just submitted a pull request and in it I just added a kwarg for table_id. I think that's a common case that is easily handled this way. As for the other metadata, I'm not sure...

Thanks for the PR! I don't think the other meta-data are important for now - the table ID is important because it's used to identify the table when reading.

…table core to better support io.table read/write integration. Adds support for identifying VOTables from their header (is_votable). Adds support for loading a single table by its ID (and ignoring all other tables). Adds support for setting the ID of the table being created using `from_table`. Adds some unit tests.

Follow on to astropy#524

mdboom · 2012-12-04T20:32:32Z

Question: I wrote this so that the extension ".xml" isn't enough to have the Table code automatically write out a VOTable file -- that is, the format needs to be explicitly set -- because "xml" is so generic. We could have this trigger on votable.xml (though I'm totally making that up) or vot (which I've seen in the wild), but most of the time people use xml for VOTable files.

I think it's ok to require users to specify the format to write tables out. I agree that xml is too generic, and that's probably the right extension to use.

mdboom · 2012-12-04T20:35:51Z

Travis is failing on some configurations. I'm looking into it...

mdboom · 2012-12-04T20:46:02Z

Ok. I think I have a fix here: https://github.com/astrofrog/astropy/pull/21/files

Fix tests on Numpy 1.4 and Python 3.x

astrofrog · 2012-12-06T00:22:37Z

I need to write the docs for this, and tidy up the API for all the readers/writers for the Table class, so will do this at the weekend.

…ndly

astrofrog · 2012-12-11T14:44:51Z

I've tidied up the docs for the table I/O, to make them more user friendly. The issue before was that I was pointing people to various API pages across the Astropy website and just telling users to go there to see what arguments could be passed to Table.read and Table.write. I've now made it so users don't have to do that anymore - the narrative should be consistent. I still point users to other parts of the docs for 'advanced information', but this is not needed for most users.

@mdboom - are you ok with the new docs? Is there anything else you think I should implement in terms of VO tables before I merge this?

@taldcroft - could you take a look at the updated docs, since I updated the section for ASCII I/O too?

mdboom · 2012-12-11T15:09:19Z

Perhaps a wild suggestion, since I'm not terribly familiar with IPAC tables, but maybe a short example would be helpful here to explain when one would use definition='left' etc.

mdboom · 2012-12-11T15:17:15Z

This looks great -- other than my documentation comments (and the suggestion of moving overwrite into the core rather than in each file type -- which of course could be a follow-on PR), I think this is good to go.

…dboom

…dboom

astrofrog · 2012-12-11T22:41:58Z

@mdboom - for now, I've ensured that the overwrite code is only executed if the output is a string. I thing we could keep the de-duplication of the logic for another PR though, as I think it needs some more thought.

mdboom · 2012-12-12T00:41:09Z

Sounds right to me.

astrofrog added 2 commits December 4, 2012 12:23

Allow VO tables to be read/written directly from astropy.table.Table …

b6be4c2

…class

Added comments

3528d7e

mdboom reviewed Dec 4, 2012
View reviewed changes

astrofrog and others added 3 commits December 4, 2012 18:01

Removed docstring that was included by mistake

b8e151d

Merge pull request #20 from mdboom/votable/table-interface

78f2a98

Follow on to astropy#524

mdboom reviewed Dec 4, 2012
View reviewed changes

Fix tests on Numpy 1.4 and Python 3.x

c234c43

Merge pull request #21 from mdboom/votable/table-interface

bb882e9

Fix tests on Numpy 1.4 and Python 3.x

astrofrog added 2 commits December 11, 2012 15:40

Fixed id -> table_id

ce43b81

Significantly expanded the Table I/O docs to make them more user-frie…

176650e

…ndly

mdboom reviewed Dec 11, 2012
View reviewed changes

astrofrog added 4 commits December 11, 2012 17:05

Added examples of IPAC tables as suggested by @mdboom

b0bc246

Added links to different table format descriptions as suggested by @m…

5fb2515

…dboom

Renamed section

47cb431

Only check for overwriting if output is a string

1099394

Conversation

astrofrog commented Dec 4, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdboom commented Dec 4, 2012

Uh oh!

mdboom commented Dec 4, 2012

Uh oh!

astrofrog commented Dec 6, 2012

Uh oh!

astrofrog commented Dec 11, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdboom commented Dec 11, 2012

Uh oh!

astrofrog commented Dec 11, 2012

Uh oh!

mdboom commented Dec 12, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants