Support other default representations in SkyCoord

@vaticancameos

Unless myself and @vaticancameos are mistaken only frames that have SphericalRepresentation as their preferred frames are supported in SkyCoord at the moment?

@Cadair

@Cadair - that's correct. What were you thinking of? Part of the issue is that the user-interface is designed to be quite flexible, but in order to make that initially manageable we have focused on spherical coordinate inputs longitude, latitude, and distance.

There is no fundamental limitation at this point so I don't imagine there being a problem supporting frames that are not spherical by default. It's just a question of making the syntax work.

Here is some documentation that is currently in work. I think it should be manageable to support cartesian coordinates as being 3-tuples of length-type quantities:

The syntax for |SkyCoord| is shown below::

  SkyCoord(COORD, [FRAME | frame=FRAME], keyword_args ...)
  SkyCoord(LON, LAT, [FRAME | frame=FRAME], keyword_args ...)
  SkyCoord([FRAME | frame=FRAME], <lon_name>=LON, <lat_name>=LAT, keyword_args ...)

**LON**, **LAT**

Longitude and latitude values.  [Fill in details].

**COORD**

A coordinate is an object that supplies one or more longitude and latitude
pairs in one of the following ways:

- Single coordinate string with a LON and LAT value separated by a space.  The
  respective values can be any string which allowed for :ref:angle-creation of
  |Longitude| or |Latitude| objects, respectively.
- A list or numpy array of coordinate strings
- A list of (LON, LAT) tuples
- An ``N x 2`` numpy or |Quantity| array of values where the first column is
  longitude and the second column is latitude, e.g. 
  ``SkyCoord([[270, -30], [355, +85]] * u.deg)``

@taldcroft

Other than @taldcroft's option of using 3-tuples, we could build our own solar-coordinates specific wrapper, @Cadair. Something like SolarCoord? Astropy's coordinates framework is pretty extensible, there shouldn't be a problem in making a new high-level class that caters to solar coordinates, instead of modifying the SkyCoord class.

Indeed it might make sense to subclass SkyCoord to re-use alot of the back-end goodies (transforms, attribute management, convenience methods etc) but rework the front-end initialization. The intent was certainly to keep much of the code (besides initialization) written in a generic (non-spherically biased) way, though it's likely some of that bias has leaked though in places.

Note also that SkyCoord is currently hardwired to the built-in frames frame_transform_graph, while for Solar you may want to be off on your own transform graph.

@vaticancameos

@vaticancameos @taldcroft I would much rather just use astropy's SkyCoord if we can, the less code we have to maintain the better ;)

Is it possible to pass a representation object into SkyCoord? I wonder if having a representation kwarg would be a good idea, as we also have to think about cylindrical coordinates as well. A lot of solar coordinates are the same 'frame' but in different representations, so having some way to specify on creation would be handy.

Dumb question: how are we currently dealing with different origins for different representations? How would we distinguish between cartesian in heliocentric vs geocentric?

@astrofrog

@astrofrog I would say that a coordinate system with a different origin is a different frame.

Is there any chance we can get a patch for this into 0.4? Just it will cause us issues if it is not upstreamed.

@eteq

@eteq - what do you think about patching SkyCoord to accept arbitrary representations?

@Cadair

@Cadair: Just saying, I'm up for patching it if need be. I have this
semester exam on 28th and then I'm in.
On May 26, 2014 7:42 PM, "Stuart Mumford" [email protected] wrote:

@astrofrog https://github.com/astrofrog I would say that a coordinate
system with a different origin is a different frame.

Is there any chance we can get a patch for this into 0.4? Just it will
cause us issues if it is not unstreamed.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2550#issuecomment-44192420
.

In terms of arbitrary representations, at first glance I think this is somewhat more of a BaseCoordinateFrame issue than SkyCoord. The existing frames like ICRS have a "matched" class attributes preferred_representation and preferred_attr_names and preferred_attr_units (see here e.g.). So right now BaseCoordinateFrame (AFAIK) was designed to have a specific representation and this isn't something you can specify on initialization.

In fact those bits currently are defined at the class level in the frame and don't get copied into instance attributes. I accidentally discovered this by modifying the preferred_attr_units in-place for a coordinate instance then being surprised when that changed units for every subsequent coordinate instance I created. (I didn't file an issue because there is not an endorsed or documented behavior).

So not to say it cannot be done, but I think it goes deeper than SkyCoord.

Just experimenting with changing the representation in a brute-force way. Kinda works, but not really...

In [24]: sc = SkyCoord(1, 2, 'icrs', unit='deg')

In [25]: sc.frame.preferred_representation = astropy.coordinates.representation.CartesianRepresentation 

In [26]: sc
Out[26]: <SkyCoord (ICRS): 999238614955 , y=0.0174417749028 , z=0.0348994967025 >

In [27]: sc.frame.preferred_representation = astropy.coordinates.representation.CylindricalRepresentation

In [28]: sc
Out[28]: <SkyCoord (ICRS): o=0.999390827019 , phi=0.0174532925199 rad, z=0.0348994967025 >

In [29]: sc.phi
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-29-8013375a2557> in <module>()
----> 1 sc.phi

/Users/aldcroft/git/astropy/astropy/coordinates/sky_coordinate.py in __getattr__(self, attr)
    325         # Fail
    326         raise AttributeError("'{0}' object has no attribute '{1}', nor a transform."
--> 327                              .format(self.__class__, attr))
    328 
    329     @override__dir__

AttributeError: '<class 'astropy.coordinates.sky_coordinate.SkyCoord'>' object has no attribute 'phi', nor a transform.

So changing the preferred_representation does not affect the
preferred_attr_names bit, evidently. That's why the phi bit throws an
error, isn't it?
On May 26, 2014 9:25 PM, "Tom Aldcroft" [email protected] wrote:

Just experimenting with changing the representation in a brute-force way.
Kinda works, but not really...

In [24]: sc = SkyCoord(1, 2, 'icrs', unit='deg')

In [25]: sc.frame.preferred_representation = astropy.coordinates.representation.CartesianRepresentation

In [26]: sc
Out[26]: <SkyCoord (ICRS): 999238614955 , y=0.0174417749028 , z=0.0348994967025 >

In [27]: sc.frame.preferred_representation = astropy.coordinates.representation.CylindricalRepresentation

In [28]: sc
Out[28]: <SkyCoord (ICRS): o=0.999390827019 , phi=0.0174532925199 rad, z=0.0348994967025 >

In [29]: sc.phi

AttributeError Traceback (most recent call last)
in ()
----> 1 sc.phi

/Users/aldcroft/git/astropy/astropy/coordinates/sky_coordinate.py in getattr(self, attr)
325 # Fail
326 raise AttributeError("'{0}' object has no attribute '{1}', nor a transform."
--> 327 .format(self.class, attr))
328
329 @override__dir__

AttributeError: '<class 'astropy.coordinates.sky_coordinate.SkyCoord'>' object has no attribute 'phi', nor a transform.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2550#issuecomment-44200383
.

It's maybe not elegant, but what will work with the current plans for 0.4 release is to define frame classes like SolarCartesian, SolarSpherical, etc. The "transform" between them should be unity, but each one will have a fixed representation.

Note that if you do something like: solar_coord.cylindrical you will get a CylindricalRepresentation object back, not a SkyCoord or even BaseCoordinateFrame object. That object will have lost all the special sauce that makes it a full-featured coordinate. So in that sense if you want easy and symmetric transforms between representations the best is to define separate frames. Then solar_coord.solar_cylindrical will give you back a full-feature coordinate with a cylindrical default representation.

@eteq

@eteq Maybe I am missing something here, but there a a couple of solar coordinate frames which share the same origin and the same information (give or take) but are in cylindrical rather than spherical or spherical instead of cartesian. Rather than writing different frames for the same system but in a different representation, I thought it would be better to get a new Frame but where the internal data is in a different representation. (Which is the behaviour solar people will expect as they see them as different systems, even though they are not really).

Basically I am suggesting a method that shortcuts this:

>>> fr2 = fr1.realize_frame(fr1.represent_as(CartesianRepresentation))

@Cadair

@Cadair - this gets to the heart of the goal for the APE5 scheme. With this approach, there is no such thing as a "frame in cylindrical" or a "frame in spherical". A frame is a frame and there are lots of ways to represent points in that frame, but they're all essentially just different basis for the same point, rather than actually different points in different reference frames.

So that's why I don't understand exactly what your goal is in changing representations for the same frame. If you have a frame that you initialized with a cylindrical representation, and want, say, the radius for the spherical version, you can just do fr1.represent_as(CartesianRepresentation).distance. That should then be completely equivalent to fr1.realize_frame(fr1.represent_as(CartesianRepresentation)).data.distance. So there should be no need to create a second frame at all.

Maybe a more complete example of what you might want to do with these two different objects would help? Maybe then I'd see that you actually want them to be more different than it seems to me?

@eteq

@eteq - maybe syntactic sugar, but I think what @Cadair wants (and what makes sense to me) is being able to do the solar equivalent of:

>>> sc = SkyCoord(..., frame='fk4', obstime=.., equinox=..)  # spherical
>>> sc.ra
..
>>> sc.obstime, sc.equinox
..
>>> sc_cart = sc.to_representation(CartesianRepresentation)
>>> sc_cart.x
..
>>> sc_cart.obstime
..  # YES, this is a real SkyCoord which has an obstime and equinox, 
..  # not a CartesianRepresentation object

That at least is my understanding of a potential use case.

(BTW, sorry, didn't have time to read the whole thread in detail, so hopefully I'm not off track or missing something that was said).

@taldcroft

@taldcroft yes that.

@Cadair

@Cadair @taldcroft - so it looks to me like the only reason why sc.to_representation(CartesianRepresentation) is necessary is to enable sc_cart.x, is that right? That is, sc.obstime and sc_cart.obstime should give the same thing.

If I'm understanding that correctly, what I'm trying to say is that this violates the separation-of-concerns APE5 is intended to implement. That is, frames should work regardless of what their representation - they may convert to the preferred representation some time, but that's not in any way intrinsic to the frame. That's way represent_as and the syntactic sugar for it exists: to allow access in any representation, not just hte preferred one.

So is the problem that sc.cartesian.x is too long to type? That works right now, because cartesian is an attribute of all of the representations...

@eteq

@eteq Ok I see your point, I forgot about the .cartesian attributes, they do make it simpler, and the separation thing is important. It does make using the .data attribute interesting for our case though as that could be in different representations relatively unknown to the user.

@eteq @taldcroft What would the best way be to patch SkyCoord to allow initialisation of simple input of different representations? I am primarily worried about passing quantities in to a frame where the preferred_representation is not spherical.

@eteq

@eteq, @taldcroft
All this stems from when I was preparing an IPython notebook detailing how to create new frames and wrap over them with SkyCoord to use them. When I set the preferred representation attribute to CartesianRepresentation, I was not able to do the wrapping bit and was getting an error.
My question is the same as @Cadair's.

@Cadair

@Cadair

Is it sufficient for you to initialize a SkyCoord using an instance of your preferred representation?
Why do you need to use .data? Within the context of APE5 I'm not sure why that's even exposed (and IIRC @eteq did not want it exposed originally). If you want cartesian .data then use .cartesian.

@eteq - I understand your point, but I think it addresses only part of the story here.

While represent_as allows access in any representation, there is currently no way to set a coordinate in an arbitrary representation except the restricted method suggested above. In particular there is no way to create a coordinate using the more natural keyword arg syntax like x=.., y=.. for a frame with default spherical representation.

Second, if you do set a coordinate with a non-preferred representation, the default repr of that object still uses the preferred one, and there is no way to change that. In the context of interactive analysis and IPython notebooks, the repr of an object is important. The repr of the cartesian representation sc.cartesian is a very different and more limited view of the sky coordinate object, and isn't sufficient in my opinion. (And I think that this is where SunPy is coming from).

In [21]: c = CartesianRepresentation(1*u.m, 0*u.m, 0*u.m)

In [22]: sc = SkyCoord(c, 'fk4', obstime='2011:005:12:13:14')

In [23]: sc
Out[23]: <SkyCoord (FK4): equinox=B1950.000, obstime=2011:005:12:13:14.000, ra=0.0 deg, dec=0.0 deg, distance=1.0 m>

In [24]: sc.cartesian
Out[24]: <CartesianRepresentation x=1.0 m, y=0.0 m, z=0.0 m>   # Missing all the context!

So even though APE5 has the concept of a preferred representation, I don't think that we all appreciated that for a particular class like ICRS, the preferred representation is hardwired and unchangeable.

So here is a concrete idea:

A frame class can define multiple representations:

class BaseCoordinateFrame(...):
    @property
    def preferred_representation(self):
        return self._preferred_representation

    @preferred_representation.setter
    def preferred_representation(self, value):
        # In reality do validation of `value` and allow string input
        self._preferred_representation = value

    @property
    def preferred_attr_names(self):
        return self._representations[self._preferred_representation]['attr_names']

    @property
    def preferred_attr_units(self):
        return self._representations[self._preferred_representation]['attr_units']

class ICRS(..):
    _representations = {SphericalRepresentation: dict(attr_names=OrderedDict([('ra', 'lon'), ...]),
                                                     attr_units={'ra': u.degree, ...}),
                       CartesianRepresentation: dict(attr_names=OrderedDict([('x', 'x'), ...]), ..),
                       CylindricalRepresentation: dict(..)}

    _preferred_representation = SphericalRepresentation

And of course because I always like string aliases I would allow setting the preferred_representation as a string like 'cartesian' (which matches the attribute access name).

The preferred_representation for a frame would be settable, but would do nothing to the internal .data.

SkyCoord would gain a new preferred_representation keyword arg as would the BaseCoordinateFrame init.

I think this is the patch that SunPy have been wanting. :-) My first impression is that it's not that hard, but I don't know if there are any hidden complications.

All - check out #2586, this is a proof of concept implementation of my suggestion above.

@Cadair

Was looking for scipy ideas, but this one seems addressed by #2586 (now merged) already. @Cadair - is this indeed the case? If so, could you close this issue?

When closing issues that are basically "won't fix" or "overcome by events" please also clear the milestone since the issue is no longer relevant to that milestone (or change it to the version the issue was actually addressed in).

Navigation Menu

Repository navigation

Support other default representations in SkyCoord #2550

In [29]: sc.phi

13 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Support other default representations in SkyCoord #2550

Description

Activity

taldcroft commented on May 23, 2014

taldcroft commented on May 23, 2014

PritishC commented on May 23, 2014

taldcroft commented on May 23, 2014

Cadair commented on May 26, 2014

astrofrog commented on May 26, 2014

Cadair commented on May 26, 2014

astrofrog commented on May 26, 2014

PritishC commented on May 26, 2014

taldcroft commented on May 26, 2014

taldcroft commented on May 26, 2014

PritishC commented on May 26, 2014

In [29]: sc.phi

taldcroft commented on May 26, 2014

13 remaining items

Cadair commented on May 26, 2014

eteq commented on May 26, 2014

taldcroft commented on May 27, 2014

Cadair commented on May 27, 2014

eteq commented on May 31, 2014

Cadair commented on May 31, 2014

PritishC commented on May 31, 2014

taldcroft commented on May 31, 2014

taldcroft commented on May 31, 2014

taldcroft commented on May 31, 2014

mhvk commented on Jul 11, 2014

embray commented on Jul 11, 2014

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions