Skip to content

Fix ECSV table example in docstring - #5263

Merged
taldcroft merged 1 commit into
astropy:masterfrom
cdeil:patch-3
Aug 30, 2016
Merged

Fix ECSV table example in docstring#5263
taldcroft merged 1 commit into
astropy:masterfrom
cdeil:patch-3

Conversation

@cdeil

@cdeil cdeil commented Aug 25, 2016

Copy link
Copy Markdown
Member

@taldcroft - This PR fixes the ECSV file example in the docstring.

It currently fails like this:

In [24]: Table.read('example.ecsv', format='ascii.ecsv')
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-24-1b015e2f57a2> in <module>()
----> 1 Table.read('example.ecsv', format='ascii.ecsv')

/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/astropy/table/table.py in read(cls, *args, **kwargs)
   2330         passed through to the underlying data reader (e.g. `~astropy.io.ascii.read`).
   2331         """
-> 2332         return io_registry.read(cls, *args, **kwargs)
   2333 
   2334     def write(self, *args, **kwargs):

/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/astropy/io/registry.py in read(cls, *args, **kwargs)
    349 
    350         reader = get_reader(format, cls)
--> 351         data = reader(*args, **kwargs)
    352 
    353         if not isinstance(data, cls):

/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/astropy/io/ascii/connect.py in io_read(format, filename, **kwargs)
     35     from .ui import read
     36     format = re.sub(r'^ascii\.', '', format)
---> 37     return read(filename, format=format, **kwargs)
     38 
     39 

/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/astropy/io/ascii/ui.py in read(table, guess, **kwargs)
    339                                              ' with fast (no guessing)'})
    340         else:
--> 341             dat = reader.read(table)
    342             _read_trace.append({'kwargs': new_kwargs,
    343                                 'status': 'Success with specified Reader class '

/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/astropy/io/ascii/core.py in read(self, table)
   1148 
   1149         # Get the table column definitions
-> 1150         self.header.get_cols(self.lines)
   1151 
   1152         # Make sure columns are valid

/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/astropy/io/ascii/ecsv.py in get_cols(self, lines)
    133 
    134         # Create the list of io.ascii column objects from `header`
--> 135         header_cols = OrderedDict((x['name'], x) for x in header['datatype'])
    136         self.names = [x['name'] for x in header['datatype']]
    137         self._set_cols_from_names()  # BaseHeader method to create self.cols

KeyError: 'datatype'

and after making this change columns -> datatype it works.

@mhvk

mhvk commented Aug 25, 2016

Copy link
Copy Markdown
Contributor

@cdeil - I think best would be to make this a docstring example, so that it gets tested automatically. But while trying that, I found I could not even get your example to work; rather I had to do:

from astropy.table import QTable
ecsv_content = """# %ECSV 0.9
# ---
# datatype:
# - {name: a, unit: m / s, datatype: int64, format: '%03d'}
# - {name: b, unit: km, datatype: int64, description: This is column b}
a b
001 2
004 3
"""
Table.read(ecsv_content, format='ascii.ecsv')
  a     b  
m / s   km 
int64 int64
----- -----
  001     2
  004     3

Looking at the original APE that described the ecsv format, I see that that is in fact shown as an example there: https://github.com/astropy/astropy-APEs/blob/master/APE6.rst

Oddly, even that is not good enough to read in with QTable:

QTable.read(ecsv_content, format='ascii.ecsv')
Out[56]: <repr(<astropy.table.table.QTable at 0x7fdf5e704828>) failed: TypeError: %d format: a number is required, not Quantity>

I think this is a separate issue with how Quantity is being formatted, but really, the example should work... cc @taldcroft.

@taldcroft

taldcroft commented Aug 25, 2016

Copy link
Copy Markdown
Member

👍 on the suggestion from @mhvk to make this a working and tested docstring example.

@taldcroft taldcroft mentioned this pull request Aug 25, 2016
Closed
@taldcroft taldcroft self-assigned this Aug 30, 2016
@taldcroft taldcroft added this to the v1.2.2 milestone Aug 30, 2016
@taldcroft taldcroft added the Bug label Aug 30, 2016
@taldcroft

taldcroft commented Aug 30, 2016

Copy link
Copy Markdown
Member

Perfect is the enemy of good, so I am merging this docs bug fix. Hopefully someone feels energetic and will make the example into a testable docstring.

@taldcroft
taldcroft merged commit b20c616 into astropy:master Aug 30, 2016
@astrobot

astrobot commented Aug 30, 2016

Copy link
Copy Markdown

@taldcroft - thanks for merging this! However, I noticed the following issue with this pull request:

  • Changelog entry not present (or pull request number missing) and neither the Affects-dev nor the no-changelog-entry-needed label are set

Would it be possible to fix this? Thanks!

This is an experimental bot being written by @astrofrog - let me know if the message above is incorrect!

@cdeil cdeil mentioned this pull request Nov 2, 2016
Merged
eteq pushed a commit that referenced this pull request Dec 21, 2016
@taldcroft @eteq
Merge pull request #5263 from cdeil/patch-3
17fdf9f 
Fix ECSV table example in docstring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@taldcroft taldcroft

Labels

Bug Docs io.ascii no-changelog-entry-needed

Projects

None yet

Milestone

v1.2.2

Development

Successfully merging this pull request may close these issues.

5 participants

@cdeil @mhvk @taldcroft @astrobot @pllim