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

Swap the order of arguments in Column from name, data, .. to data, name, .. #840

Merged
merged 4 commits into from Mar 6, 2013

Conversation

taldcroft
Copy link
Member

Table Column and MaskedColumn will now have the initializer signature

def __new__(data=None, name=None, ...)

This was discussed in #726 and then made possible as a smooth transition by #731. In particular #731 (which is in 0.2) issues a warning if users call Column with an initial column name arg value instead of using keyword args.

In this PR there is some compatibility helper code. If the user supplies an initial arg value which is a string (corresponding to the pre-0.2 method), this raises an exception with a useful message:

>>> Column('a', [1, 2])
...
ValueError: 
The first argument to Column is the string 'a', which was probably intended
as the column name.  Starting in Astropy 0.3 the argument order for initializing
a Column object is Column(data=None, name=None, ...).

@ghost ghost assigned taldcroft Mar 1, 2013
@taldcroft
Copy link
Member Author

Can someone have a look at this?

I'd like to get this in soon because the doc updates for #726 depend on it, so this is blocking progress there.

@astrofrog
Copy link
Member

@taldcroft - this looks good to me! One minor comment, maybe in generate_ref_ast the arguments should also be switched around, and data= removed? (as in other places in the code?)

@taldcroft
Copy link
Member Author

With the changes in #726, the syntax in generate_ref_ast will be obsolete anyway. All those would be better written like t['obstime'] = obstime, so I'm inclined to just leave generate_ref_ast.py alone for now. My guideline has been that documentation must use the preferred syntax, but test code only has the requirement of working.

@astrofrog
Copy link
Member

@taldcroft - sounds good - in that case, feel free to merge. Looking forward to the awesome new syntax!

taldcroft added a commit that referenced this pull request Mar 6, 2013
Swap the order of arguments in Column from name, data, .. to data, name, ..
@taldcroft taldcroft merged commit bcce7c5 into astropy:master Mar 6, 2013
@taldcroft taldcroft deleted the table/swap-column-args branch March 6, 2013 16:32
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.

None yet

2 participants