Repository navigation
Should coordinates be given in "preferred units" or as passed in by user #4903
Description
This separates out the more contentious question in #4883, whether it is a "good thing" that coordinates are automatically converted to preferred units for presentation in c.ra, etc. The main argument pro seems to be that some people may now count on coordinates always being in degrees; the main arguments counter are that for large arrays, this is a non-trivial memory overhead for no useful purpose (as the original input is what is actually used in any calculations); and that typically you if you enter something in some units, you should expect to get your data back that same way if that is trivially possible, i.e., SkyCoord(ra, dec).ra is ra (or at least the underlying memory is shared).
For reference, currently:
In [6]: t = SkyCoord(ra=np.arange(10) * u.hourangle, dec=np.arange(10) * u.deg)
In [7]: t.frame.data.lon # this is the original data
Out[7]: <Longitude [ 0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] hourangle>
In [8]: t.ra # converted for me without my request
Out[8]: <Longitude [ 0., 15., 30., 45., 60., 75., 90., 105., 120., 135.] deg>
Activity
taldcroft commentedon May 27, 2016
@mhvk - the big downside in my mind to your proposal is for the very common use case of inputting a coordinate in sexigesimal format, in which case the
raattribute would end up being in hourangle. This would be very inconvenient and annoying to me, and I imagine many users. I believe that degrees is far and away the most common unit for decimal representation of sky coordinates in astronomy.Maybe we could add an attribute
raw_repror something that disables this conversion for memory-intensive applications?taldcroft commentedon May 27, 2016
Or plan B would just be documenting this behavior and saying "if you have a memory-intensive application then you should consider using
coord.data.lon/latinstead ofcoord.rato avoid making a copy of the coordinate values".mhvk commentedon May 27, 2016
Obviously, a matter of taste: I do like hour angles... I know where I am along the galactic plane in hours, but not in degrees... I'd say that most reasonable is to have a default of staying as close as possible to what the user put. But of course this harkens back to your comment that really one should display things the way they were put in...
But failing that, I don't have any problem with converting to degrees if one has to convert
dtypeanyway (from string, tuple, etc). I just don't think that what the code uses internally should be hidden... Plus it causes unnecessary overheads. For instance, inreshape, etc., I now have to remove items from_rep_cache(or delete and recreate it).mhvk commentedon May 27, 2016
Also, to be a bit more specific: is it the fact that the values of
.raare inhouranglethat is annoying to you, or the fact that it is displayed in hour angle? I.e., would your annoyance be solved by having a way to set the defaultrepr, or do you feel the actual values should be in degrees? I ask because I very much share your wish in having proper format support for angles and coordinates -- I would likerepr(sc)to be able to give me RA in hms strings (I love the flexibility we have for that inTime).taldcroft commentedon May 27, 2016
The fact that the values of
.rawould be inhouranglewould be annoying to me. But more importantly,astropy.coordinateshas beenstablesince astropy 0.4, and that includes the API decision at the time that.rawill have a default and consistent unit ofdegrees. Changing this is a big deal and will break a lot of code in the wild. Even though you and I know better, I have no doubt that you would find plenty of lines likera = coord.ra.valueout there.So adding support for formats is (in my opinion) the only reasonable path forward at this point, especially since what you really want is HMS-style default repr. I think that even right now you can change your default repr unit for a particular instance or via subclass, though it's been a while since I looked at this.
One question about the need for manipulating
_rep_cacheinreshape-- shouldn'tcoord.reshape(...)return a new SkyCoord with an already-empty_rep_cache? So why do you need to fiddle with it? Sorry I'm being too lazy to go and look at the code, but I did remind myself howTimeworks and basically it's not required there for the scale and format caching because_replicatedoes not propagate the cache to the new object.eteq commentedon May 28, 2016
I agree with @taldcroft here about the "API is stable" bit. I recall plenty of debating about this (although I honestly don't even recall what, if any, my opinion was at the time...), and unless something else has actually changed in the meantime, I don't think altering this is wise.
But if I'm understanding right there are now two items under discussion:
sc.raif you've decided you don't care about the units - this is to improve performance. I agree this is something we should try to address, but it should either be "opt-in" (to prevent the API-breaking), or something like what @taldcroft suggested in the direction of "clearer documentation".(I presume it wouldn't look exactly like that because of the need for frame info, array support, and such, but is that the general idea?)
eteq commentedon May 28, 2016
Oh, and here's an idea for the
#1I have above: what about something like.raw_ra/.raw_dec? Those could be made on-the-fly just the way.ra/.decare right now, but the key difference would be they are in whatever the underlying units are, with no rescaling.adrn commentedon Jun 12, 2017
A possible solution, as @eteq just mentioned, could be to add an optional kwarg (
convert_to_preferred_units?)mhvk commentedon Jun 12, 2017
closing in favour of #6184
taldcroft commentedon Jun 12, 2017
@mhvk @adrn @eteq @taldcroft agreed that we are not willing to change this default behavior at this point, so closing now.