Skip to content

Allow specifying differential class in frame init - #6317

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

adrn merged 3 commits into
astropy:masterfrom
adrn:coordinates/diff-class-init

Conversation

@adrn

@adrn adrn commented Jul 2, 2017

Copy link
Copy Markdown
Member

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

@eteq eteq left a comment •

Copy link
Copy Markdown
Member

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?

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

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

adrn commented Jul 5, 2017

Copy link
Copy Markdown
Member Author

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