Skip to content

Allow specifying differential class in frame init#6317

Merged
adrn merged 3 commits intoastropy:masterfrom
adrn:coordinates/diff-class-init
Jul 5, 2017
Merged

Allow specifying differential class in frame init#6317
adrn merged 3 commits intoastropy:masterfrom
adrn:coordinates/diff-class-init

Conversation

@adrn
Copy link
Member

@adrn adrn commented Jul 2, 2017

Fixes #6314

This adds support for specifying a differential class in the frame intializers, which should mirror the way passing in a representation class works. This supports, e.g.:

>>> icrs = coord.ICRS(ra=8.67*u.degree, dec=53.09*u.degree,
...               pm_ra=4.8*u.mas/u.yr, pm_dec=-15.16*u.mas/u.yr,
...               radial_velocity=23.42*u.km/u.s,
...               differential_cls=coord.SphericalDifferential)
>>> icrs 
<ICRS Coordinate: (ra, dec) in deg
    ( 8.67,  53.09)
 (pm_ra, pm_dec, radial_velocity) in (mas / yr, mas / yr, km / s)
    ( 4.8, -15.16,  23.42)>

(also differential_cls={'s': coord.CartesianDifferential})

This also decouples setting the representation class from setting the differential class, so this is now valid as long as the differential class is compatible with the representation:

icrs.set_representation_cls(s=coord.SphericalCosLatDifferential)

@adrn adrn added this to the v2.0.0 milestone Jul 2, 2017
@adrn adrn requested review from eteq and mhvk July 2, 2017 20:53
@bsipocz bsipocz added Affects-dev PRs and issues that do not impact an existing Astropy release Bug labels Jul 2, 2017
Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

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

LGTM, @adrn, except shouldn't there now be a differnential_cls section in the Paramaters part of the baseframe dosctring?

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.

(from holidays, no brief) I had removed this more or less on purpose, with the logic was that you should never be able to set a differential class inconsistent with a base class anyway, so that, e.g., sphericalcoslat would just set the differential class and representation. but I do see that this makes unitspherical impossible, and this seems a reasonable solution. Still not 100% sure it is necessary to have this short-cut to passing a dict to representation, but only -0, so since you both feel it is worth it, OK to proceed.

@adrn
Copy link
Member Author

adrn commented Jul 5, 2017

Approved and passing - hitting merge!

@adrn adrn merged commit f89f3d9 into astropy:master Jul 5, 2017
@adrn adrn deleted the coordinates/diff-class-init branch July 5, 2017 01:06
bsipocz pushed a commit that referenced this pull request Jul 5, 2017
Allow specifying differential class in frame init
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release Bug coordinates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants