Support other default representations in SkyCoord

[Cadair]

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

[taldcroft]

@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.

[taldcroft]

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)``

[PritishC]

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.

[taldcroft]

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.

[Cadair]

@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.

[astrofrog]

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

[Cadair]

@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.

[astrofrog]

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

[PritishC]

@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" notifications@github.com 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
.

[taldcroft]

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.

[taldcroft]

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.

[PritishC]

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" notifications@github.com 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
.

[taldcroft]

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.

[taldcroft]

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?

@vaticancameos - I suspect it's deeper than that in the sense that the internal representation (i.e. the table of numbers that comprise the coordinate data) is really spherically-based. Not clear what sort of unexpected things happen when the representation is just abruptly changed.

[taldcroft]

And who knows, maybe it's not that hard to add an update_representation method to BaseCoordinateFrame to do this in-place. That's a question for @eteq.

[eteq]

Sorry, I missed that this discussion was happening. A few separate points:

• Different systems with different origins are different frames. So the answer to @astrofrog's question is: they should be different frame classes. Then just define a transformation between the two that switches to cartesian, offsets by whatever the origin is, and then returns an object in the new system.
• Also, frames are immutible. So definitely you should not be changing preferred_representation like in @taldcroft's snippet above. Instead, just define a new coordinate frame with its own preferred_representation.
• SkyCoord absolutely should support all kinds of representations and frames. So I really don't want there to be any reason to create a SunpyCoord. The whole idea of having the frame classes with a preferred_representation was that SkyCoord could just defer to the frame's initializer to figure out what to do. I actually thought this is what SkyCoord is doing already, but I guess that's not the case, @taldcoft? Something like the snippet below is what I thought should work:

class SomeFrame(BaseCoordinateFrame):
    preferred_representation = CylindricalRepresentation #doesn't exist yet - assuming sunpy will do it
    frame_attr_names = {'obstime': Time('J2000')}

SkyCoord(phi=8*u.deg, z=0.5*u.kpc, r=0.95*u.AU, frame=SomeFrame())

So the idea is that SkyCoord would first try to do the parsing it does now for strings and equatorial systems and the like, and if it fails, it falls back on just passing into frame.realize_frame the relevant kwargs. @taldcroft - do you see any reason why this can't/shouldn't work?

If that worked, would it address your use cases, @Cadair and @vaticancameos ?

[taldcroft]

@eteq - from what I can tell the code you suggested doesn't register SomeFrame in baseframe.frame_transform_graph, and SkyCoord is tightly integrated with that particular transform graph. If the required frame isn't in that graph then SkyCoord fails. One reason that's needed is that SkyCoord needs to know in advance if an attribute belongs to any allowed frame in the transform graph. That is fairly core to the design. That's why I was thinking about SunPy subclassing SkyCoord and putting their frames into a distinct transform graph.

I can certainly imagine things working more nicely, but I'm trying to think of things that would work with the coordinates we have now. We're starting to run into the type of suggestions that could noticeably delay release. We really must have astropy 0.4 out by SciPy, which I think realistically means focusing 100% on tidying the docs, finding bugs, getting all the packaging issues sorted, and the 500 other things that will come up. So my vote is not to accept new feature requests at this time.

To be honest I think that SunPy are not the only ones that will have good ideas for features once the new coordinates is released. Is there a prohibition against having a mid-cycle 0.5 release for coordinates before the regularly scheduled 1.0?

[taldcroft]

Just FYI, this works right now for initialization:

In [4]: SkyCoord(CartesianRepresentation(1*u.m,0*u.m, 0*u.m))
Out[4]: <SkyCoord (NoFrame): ra=0.0 deg, dec=0.0 deg, distance=1.0 m>

In [5]: SkyCoord(CylindricalRepresentation(1*u.m,1*u.deg, 0*u.m))
Out[5]: <SkyCoord (NoFrame): ra=1.0 deg, dec=0.0 deg, distance=1.0 m>

[Cadair]

Hello,

Let me try and work this out. This issue seems to have gone a little larger in scope than I intended. I am fully aware that we are on a deadline for 0.4, all I am really asking to get into 0.4 is the ability to support different prefered reprsentations. We are happy to contribute code, but it might be a good idea if you tell us a good plan of attack.

To answer more specific points:

@taldcroft We are planning on putting the SunPy frames onto the Astropy graph, because in theory we should be able to transform to the coordinate systems in Astropy, while it will be a neiche use case, it will be a cool feature.

@eteq I think your example would service our needs perfectly.

Another related question, why does doing this:

x = SunPyFrame(...) #defaults to cartesian
x.represent_as(UnitSphericalRepresentation)

return a representation object rather than a full frame in a new representation? The latter behaviour is going to be needed for us, in one way or another. (I realise you could stitch it together, but that is quite ugly).

@taldcroft Your last example means we can at least make something work.

We are going to be breaking this API this summer, we are going to be doing a lot of things that you might not have planned for. Part of the plan is to push this code back up and deal with these issues, a 0.5 release might not be a bad idea ;)

[eteq]

@taldcroft - ah, you're right, I forgot to add a transform:

frame_transform_graph.transform(FunctionTransform, SomeFrame, ICRS):
def someframe_to_icrs(somecoo, icrsframe):
    rep = somecoo.represent_as(CartesianRepresentation)
    newrep = CartesianRepresentation(rep.x+offset_x, rep.y+offset_y, rep.z+offset_z)
    return icrsframe.realize_frame(newrep)

That will include it in the frame transform graph and allow you to go to/from SomeFrame to ICRS. Then it should work in SkyCoord, right?

[eteq]

@Cadair - represent_as yields a new representation everywhere - that's specifically it's purpose. @taldcroft had a similar comment earlier to what you said here for SkyCoord, so it might be that this method should have a new name that makes it clear what it is. Perhaps get_representation or something like that? (That's an easy change because it's just find-and-replace).

But I don't quite understand what you mean when you say "rather than a full frame in a new representation". Representations do not have "intrinsic" representations - the preferred representation is just the most common one used by that frame. So if you have some frame that has a preferred cartesian representation, the way to actually get the spherical representation is exactly what you showed above. Internally, it's cached the first time you ask for the spherical representation, so it's not like it's wasting time re-computing it every time.

[eteq]

Oh, and to address

We are planning on putting the SunPy frames onto the Astropy graph

That's exactly the way this is intended to be used: all transformations of spatial/celestial coordinates should live on the same transform graph. We'll only start needing multiple graphs when gWCS gets implemented.

[Cadair]

@eteq Ok so I think this might be a better idea:

[Note I did this from memory]:

>>> fr1 = MyFrame(...)
>>> type(fr1.data)
UnitSphericalRepresentation(...)

>>> fr1.get_represenation(CartesianRepresentation)
CartesianRepresentation(...)

>>> fr2 = fr1.represent_as(CartesianRepresentation)
>>> type(fr2)
MyFrame(...)

>>> type(MyFrame.data)
CartesianRepresentation(...)

[Cadair]

@eteq correct me if I am wrong but at the moment to get a new frame in a different representation you would have to do:

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

????

[eteq]

@Cadair - I don't quite understand what you mean by "get a new frame in a different representation". I would say the way to do that is to just do:

fr1 = MyFrame(...)
myrepr = fr1.represent_as(CartesianRepresentation)

That is, you only change representations when you actually want to do something with them. The internal representation used by a frame object should generally be regarded as an implementation detail... Can you be more specific about why you want it to be a particular representation?

[eteq]

Oh, but that question aside, there's a slightly better way to acheive the same aim:

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

that will then preserver the frame attributes from fr1

[Cadair]

@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))

[eteq]

@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?

[taldcroft]

@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).

[Cadair]

@taldcroft yes that.

[eteq]

@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...

[Cadair]

@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.

[PritishC]

@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.

[taldcroft]

@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.

[taldcroft]

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.

[taldcroft]

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

[mhvk]

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?

[embray]

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).