Milestoning as 1.0 since I think this will be critical to have if we merge #3011

I'm good with this, except that I think the default should always be decimal. This provides a consistent and easily-machine parseable output for any situation. For me consistency is the key, and having str(c) always match c.to_string() makes life simple instead of allowing certain special cases. And of course X-ray astronomers don't hobble themselves by going back-and-forth to sexigesimal. 😄

I think we had this argument about always decimal vs. frame-dependent style before and I managed to convince people to stick with decimal always as the default repr of coordinate values.

@taldcroft - ok I don't mind if we keep the current to_string behavior but it would be good to think about how we could allow the user to control the format when writing to a table. Maybe we could envisage having the default for SkyCoord be modifiable, e.g.

In [1]: c.to_string()
Out[1]: '10.6847 41.2687'

In [2]: c.style = 'hmsdms'

In [3]: c.to_string(style='hmsdms')
Out[3]: '00h42m44.33s +41d16m07.5s'

This would then allow users to customize the output style used for Table.write if that simply uses str(c). I'm fine with deferring that decision for now since it would be backward compatible.

In any case, it sounds like you agree that str should connect to to_string?

Yes, str should connect to to_string.

I'm totally in favor of having the formatting of SkyCoord be controlled by some object attribute(s)! I basically argued this point until I was blue in the past (without quite getting any traction), in part because I was expecting that table mixins would be a reality. In this case it becomes obvious that a coordinate object needs an internal and persistent knowledge of how to format itself. By persistent I mean that if you slice a coordinate then that formatting attribute needs to get inherited.

I agree that the way we implement this will take some discussion and almost certainly cannot make 1.0.

One slight hiccup with linking str to to_string - to_string actually does not return a string but a vector of strings for vector coordinates. What would be the best way to deal with this? Join with newlines?

One option is to simply raise an error if str() is used on a vector SkyCoord and tell the user to use to_string instead.

👍 to linking __str__ to to_string(). But whatever the default is, it should include the sign for the declination!

On multiple coordinates: I think one string presented as a list of strings is the best solution. How about just returning str(self.to_string())? This is what Time effectively does internally as well.

p.s. We should try to define default formats consistently through astropy. Time can do it, but has several knobs instead of just one (out_fmt, precision, ...).. My preference would be to define __format__ properly in each class and then have an associated attribute that stores the default format string that is used (just like a float implicitly has 'f' as a default format string). There was a long discussion about implementing __format__ for coordinates on astropy-dev; so far, it has been implemented in Unit and Quantity (though the latter gives only partial control). Anyway, for another PR...

One issue I found is also that the default to_string doesn't show the distance, but if this is a full part of the coordinate maybe it should be. But I don't have strong feelings.

+1 for consistency across Astropy. I like the format functionality of Unit and Quantity and we should have more of that!

I've now changed this to return str(SkyCoord.to_string) which means that it works for array output too.

@astrofrog - I realise it is slightly beyond the narrow scope of this PR, but I think for str we really should ensure the latitude carries a sign! I.e., in sky_coordinate.to_string, can we add 'alwayssign': True to dms and decimal too? (I have no idea why this is not the default; pinging @eteq).

p.s. The travis failures are for unicode issues for python2. From a previous exchange I vaguely remember that one should not use unicode_literals any more, but it may also be as simple as adding a str() around the desired part in the assert statement.

Damn this Python 2 nonsense. How long are we going to have to support that archaic version? 😛

Hmm, there are three aspects of this that concern me:

First, I think the current implementation is problematic in how it deals with units - that is, it doesn't show them at all. For Time this is a non-issue because there's enough context to make it clear what the units are... but that's not true for coordinates. A simple fix might be to just add 'deg' (or 'd') to the end if we're going to actually require that it always be decimal degrees.

Second, I think it's also an issue to always require decimal degrees. The following example illustrates the problem:

sc = SkyCoord(x=1*u.km,y=2*u.km,z=3*u.km,frame='itrs')
>>> str(sc)
'63.4349 53.3008'

This is happening because the ITRS frame defaults to a Cartesian representation, not Spherical, but to_string is making it into decimal degrees. That certainly seems like unexpected behavior. So instead I would think at least the units/representation should be the same as what repr gives. (Note that this makes the first problem more pressing in that different outputs will have different units)

Third, I'm worried that this now drops all information about what the frame is. As @taldcroft and @astrofrog said, there's precedence in Time in that the scale is dropped. But I can't shake the feeling that this is much more important in coordinates, as the different components might mean something entirely different (whereas in Time, the different time scales are relatively subtle differences of a similar physical quantity). A possible solution might be to have __str__ give out something like 'ICRS: 1 2' or 'Galactic: 12.1 34.6' or similar... That's now getting pretty far afield of what to_string gives, though, so this point I can be persuaded against if the rest of you really aren't worried about this.

@eteq - I share some of those concerns. At the end of the day I think what I would like is to make sure that the output from tables looks sensible. The issue with having the frame in the line with the coordinates is that a table would then look like

          coords
--------------------------
   Galactic: 12.1 34.6
   Galactic: 12.2 34.6
   Galactic: 12.3 34.6
   Galactic: 12.4 34.6
   Galactic: 12.5 34.6
   Galactic: 12.6 34.6

Having said that I think that 12.6 34.6 is maybe too little so there is a balance to achieve here.

@eteq - definitely some nails on the head, there! I do think the automatic conversion to a pair of angles in degrees is really incorrect, given your example. It is also inconsistent with what Time does: it doesn't just always convert to jd, but respect what the user put in. So, I think logically, we should keep cartesian if that was what was entered, and hexagesimal if that was what was put in, etc. I'd also advocate that if just numbers are presented, the unit is kept.

That said, I'm much less worried about the system: I think it is for the user to know what system they used; again, there is precedence in Time in not automatically converting to anything different from what the user put in, but also not worrying about indicating the system:

In [195]: str(Time.now())
Out[195]: '2015-01-21 21:10:30.074440'
In [198]: str(Time(Time.now(), format='mjd'))
Out[198]: '57043.88259317037'

In [199]: str(Time(Time.now(), format='cxcsec'))
Out[199]: '538261928.19372'

Overall, I fear that @taldcroft is right, and SkyCoord will have to keep this state (or perhaps Angle has to...). I'd be inclined to postpone this beyond 1.0 for that reason, though can see arguments as well for it being at least a little better with the PR (though, please, at least add the sign to latitude!).

p.s. In the table context, I'd hope both system and units could be listed as the "unit".

I've removed the milestone since this is going to require more discussion. And I agree with @mhvk that whatever happens, the latitude should include a sign ;)

Repository health increased by 10% when pulling 8d2bae8 on astrofrog:skycoord-str into 0233c22 on astropy:master.

• 274 new problems were found (including 6 errors and 263 code smells).
• 1554 problems were fixed (including 1245 errors and 192 code smells).

Hi humans 👋 - this pull request hasn't had any new commits for approximately 2 years. I plan to close this in a month if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please close this and open an issue instead to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If you believe I commented on this issue incorrectly, please report this here.

Closing this in favour of new issue #6617