ping @astrofrog

@eteq - there are a few changes compared to the API document:

• PhysicistSphericalRepresentation is PhysicsSphericalRepresentation
• representation=... is now a class method instead, simplifying the code a lot
• We've added the unitspherical representation

Transformations are implemented in a simple way - by default, all transformations go through cartesian, but one can add short cuts by overloading represent_as. It's simple, but it works well. We still need to overload some of the represent_as methods for e.g. UnitSpherical but that's just optimization, and doesn't change the functionality.

Also, what exactly is the physics spherical representation? At the moment, it simply renames lon to phi and lat to theta, but should theta = 90 - lat?

@Cadair - my fixes are in Cadair#1

I'm going to punt on the __getitem__ for now. I suggest we try and get this in already and then open a new pull request to add other features such as slicing.

@astrofrog - I'm fine on punting on __getitem__ in this PR, but are you fine with the API plan of including this? Its relevant because if so, I can just delegate __geitem__ from the frame classes to the representations. (And presumable the high-level classes can do the same to the frame classes, @taldcroft)

@eteq - absolutely, I think slicing should work. Do we want to allow scalar representation objects, and if so, should slicing just return an exception?

@astrofrog - yeah, I would say so... CartesianRepresentation(1,2,3) should be different from CartesianRepresentation([1], [2], [3]), after all. I think you can get away with all of the representations doing something like:

def __getitem__(self, slc):
    return self.__class__(self.x[slc], self.y[slc], self.z[slc])

or similar, and then let the initializer decide if that's valid? But maybe its more complicated then that...

@Cadair - see Cadair#2 for some fixes and implementation of __getitem__ - it was as simple as @eteq pointed out.

@astrofrog - agree to keep it simple for now. My guess would be that in the end we want an underlying vector class, which also allows addition, etc.

I was also thinking of writing a quick __str__ and __repr__; where would I send a PR?

@mhvk send a pull request to Cadair/astropy:ape5-repr

@mhvk @astrofrog - I'm actually marginally against implementing __add__ and such because it won't always mean the same thing for different representations (for Cartesian it might make sense, but everything else is ambiguous). But I agree we can decide that later.

__getitem__ does need to be in for 0.4, though, if we want to have the catalog-matching stuff in the high-level classe. That's pretty useless without 1D indexing (which means we also need it in the frame and representation classes).

EDIT: Oh, I see @astrofrog added that already - awesome!

I think I'm missing something, but did we ever discuss adding __add__? I agree it should not be added for now.

@astrofrog - thats what I thought @mhvk was saying in his last comment? And are you done making changes now, @astrofrog and @Cadair ? If so, I can merge this and @mhvk can issue a PR against eteq/astropy:coordinates-ape5 . That will make it easier to start hooking this into the frames stuff.

Ah I see - sorry for the confusion. I need to make one more commit to address a comment @mhvk made, then we are good to go.

@Cadair - can you merge in Cadair#3

No worry about __add__ for now. I'll make a PR. My sense would be that __mul__ is more important (add distances). Anyway, all for later.

@eteq - this is good to merge!

Alright, merging now! @mhvk - you can issue your __str__ and __repr__ to my eteq/astropy:coordinates-ape5 , then.