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

Init signatures for coordinate frames could be improved #7709

Open
adrn opened this issue Aug 6, 2018 · 7 comments
Open

Init signatures for coordinate frames could be improved #7709

adrn opened this issue Aug 6, 2018 · 7 comments

Comments

@adrn
Copy link
Member

adrn commented Aug 6, 2018

The "init signature" for a function / class is displayed in IPython with the docstring when using ? or tab-completing within open parentheses. For the coordinate frame classes, the init signatures typically just display *args instead of listing the names of the default coordinate components expected, e.g., for ICRS:

Init signature: coord.ICRS(*args, copy=True, representation_type=None, differential_type=None, **kwargs)

instead of

Init signature: coord.ICRS(ra, dec, distance, pm_ra_cosdec, pm_dec, radial_velocity, copy=True, representation_type=None, differential_type=None, **kwargs)

The reason is that IPython is doing some inspection to determine the init signature, and the frame classes have very flexible initialization schemes. For example, one could pass in the components or a Representation object, and the component names depend on the setting of representation_type / differential_type.

See also discussion in #7708

cc @bsipocz @eteq @mhvk

@mhvk
Copy link
Contributor

mhvk commented Aug 6, 2018

A partial solution might be to at least enumerate all the possibilities as keyword arguments, i.e., something like ICRS(*args, ra=None, dec=None, representation=None, copy=...)

Though as I noted in the other thread, I think it may be most important to try to think through how to deal with SkyCoord, since most users will be using that instead of the frame classes.

@Cadair
Copy link
Member

Cadair commented Aug 14, 2018

My opinion has been for a while that the signatures for the frames and SkyCoord are way too lenient, and it would be much better to make use of the new keyword only arguments etc in Python 3 to make them more accessible, at the cost of flexibility.

@eteq
Copy link
Member

eteq commented Aug 20, 2018

@Cadair

My opinion has been for a while that the signatures for the frames and SkyCoord are way too lenient, and it would be much better to make use of the new keyword only arguments etc in Python 3 to make them more accessible, at the cost of flexibility.

I think I agree for the frames, but SkyCoord is supposed to be lenient - that's the main reason for it's existence, to allow more lenient parsing. Or do you mean specifically that we should not allow SkyCoord('1:2:3', '4:5:6')?

@eteq
Copy link
Member

eteq commented Aug 20, 2018

I should also point out @maxnoe's #7708 (comment) & #7708 (comment), though: the argument there is to use classmethods.

I'd riff on that just a bit further to offer an alternative idea: have the frame classes (and maybe SkyCoord?) actually do all the work in new classmethods, and have the initializer be a relatively simple multiple-dispatch to the classmethods to essentially provide backwards compatibility?

Thats sort of backwards of how one typically thinks of initializers (i.e., classmethods and "extra" ways and the initializer is the "basic" way)... But might be the least bad solution here?

@mhvk
Copy link
Contributor

mhvk commented Sep 14, 2018

Am adding here from #7708 (comment) the idea that some of the flexibility can and probably should be moved to class methods (such as from_representation).

@Cadair
Copy link
Member

Cadair commented Sep 17, 2018

Also if all the actual loading were moved into class methods, if you needed performance at the expense of flexibility (i.e. you knew the form of your input) you could use the class method and bypass a lot of code checking?

@mhvk
Copy link
Contributor

mhvk commented Sep 17, 2018

@Cadair - agreed - the class methods would be both clearer and faster.

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

No branches or pull requests

4 participants