Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set units and descriptions in Table init #9671

Merged
merged 5 commits into from
Nov 26, 2019

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Nov 25, 2019

This allows explicitly setting both the units and descriptions of specified table columns from the Table.__init__() and Table.read() methods.

Closes #9639

It provides some mitigation for #9665 (read units row from ASCII table) by making it easier to do that:

    text = ['a,b',
            ',s',
            '1,2',
            '3,4']
    units = Table.read(text, format='ascii', data_start=1, data_end=2)[0]
    t = Table.read(text, format='ascii', data_start=2, units=units)

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks good. And would propagate automatically?

astropy/table/table.py Outdated Show resolved Hide resolved
@taldcroft
Copy link
Member Author

I added kwargs to Table.read as well. In theory this is ready for final review.

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good - only a total nitpick (which I think may matter for sphinx) and what I believe is an incorrect example. Probably good to turn on testing!

astropy/table/table.py Outdated Show resolved Hide resolved
docs/table/construct_table.rst Outdated Show resolved Hide resolved
@bsipocz bsipocz added this to the v4.1 milestone Nov 26, 2019
@taldcroft taldcroft changed the title WIP set units and descriptions in Table init [skip ci] Set units and descriptions in Table init Nov 26, 2019
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good! Really only some missing periods, which I am not even sure are important...

@@ -33,6 +33,10 @@ class TableRead(registry.UnifiedReadWrite):
first argument is typically the input filename.
format : str
File format specifier.
units : list, dict, optional
List or dict of units to apply to columns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the periods at the end here too....

if not values:
return

if isinstance(values, Row):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future, perhaps we should have Rows.items() so there isn't a need to special-case it.

# For a Row object transform to an equivalent dict.
values = {name: values[name] for name in values.colnames}

if not isinstance(values, dict):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might even register Row as collections.abc.Mapping, and then do the instance on that (I'm a bit puzzled it is considered a Sequence now, since one cannot index with the column number anyway; similarly, Row.__iter__ is weird.

But really out of scope here! See #9687

@taldcroft taldcroft merged commit aad16d6 into astropy:master Nov 26, 2019
@taldcroft taldcroft deleted the table-unit-init branch November 26, 2019 17:55
@pllim
Copy link
Member

pllim commented Nov 26, 2019

@adrn , now you can do it with dev astropy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow specifying a list of column units to io.ascii.read()
4 participants