Skip to content

Can't specify differential class in frame init #6314

Closed
Closed
Can't specify differential class in frame init#6314
Labels
Affects-devPRs and issues that do not impact an existing Astropy releaseBugcoordinates
Milestone
v2.0.0

Description

@adrn
adrn
opened on Jul 2, 2017

One of the nice things about the frame classes is that you can specify a representation class, and the expected kwarg names change accordingly:

>>> icrs = coord.ICRS(x=1*u.pc, y=2*u.pc, z=3*u.pc, 
...                   representation=coord.CartesianRepresentation)
>>> icrs
<ICRS Coordinate: (x, y, z) in pc
    ( 1.,  2.,  3.)>

I had originally made it so that this would work with a differential class as well, but that no longer seems to be the case (because of some changes @mhvk made in #6219). An implication of this is that the velocity components always have to be specified with the component names corresponding to the default differential. For most frame classes, this is SphericalCosLatDifferential, so proper motions always have to be coslat. However, it would be nice to be able to do something this:

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,
                  differential_cls={'s': coord.SphericalDifferential})

when someone doesn't want to include the cos(dec).

Or, for cartesian components:

icrs = coord.ICRS(x=1*u.pc, y=2*u.pc, z=3*u.pc, 
                  v_x=10*u.km/u.s, v_y=11*u.km/u.s, v_z=12*u.km/u.s,
                  representation=coord.CartesianRepresentation,
                  differential_cls={'s': coord.CartesianDifferential})

cc @eteq @mhvk

Activity

  1. adrn
    added
    coordinates
    on Jul 2, 2017
  2. adrn
    added this to the v2.0.0 milestone on Jul 2, 2017
  3. eteq
    added
    Bug
    Affects-devPRs and issues that do not impact an existing Astropy release
    on Jul 2, 2017

  4. eteq commented on Jul 2, 2017

    @eteq
    eteq
    on Jul 2, 2017
    Member

    I agree with @adrn that this is functionality that should be present. I think it's fair to call it a bug that this isn't possible, because it's definitely intended (and highly desirable).

    In my mind, the first example @adrn gave above is a critical piece of functionality, because there is no agreed-upon convention for how this is represented in the field.

  5. eteq commented on Jul 2, 2017

    @eteq
    eteq
    on Jul 2, 2017
    Member

    One other thing this issue has bad me realize: ICRS(..., differential_cls=SphericalDifferential) apparently no longer works (it did in a previous cut @adrn and I made on this functionality). I think that also needs to be supported (in addition to the dictionary syntax), as the dictionary syntax is a relatively arcane implementation detail. While it's necessary for higher-order differentials, we the frames are already specially-tailored to work for just velocity differentials. (As we should, because they are coordinate frames not arbitrary vector frames.)

  6. adrn
    closed this as completedin #6317on Jul 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Affects-devPRs and issues that do not impact an existing Astropy releaseBugcoordinates

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions