Repository navigation
Implement a DistanceOffsetFrame that's like SkyOffsetFrame but includes distance effects #7770
Description
From the documentation of SkyOffsetFrame objects:
A frame which is relative to some specific position and oriented to match its frame.
So I hoped I would be able to do things like this:
GCRS2 = SkyOffsetFrame(origin=ICRS(get_body_barycentric("earth", Time.now())))
However, it seems that SkyOffsetFrame objects are not using the distance information, and transformations with or without it produce the same result:
In [3]: skyoffset_nodist = SkyOffsetFrame(origin=ICRS(ra=31 * u.deg, dec=10 * u.deg))
In [4]: skyoffset_dist = SkyOffsetFrame(origin=ICRS(ra=31 * u.deg, dec=10 * u.deg, distance=1 * u.kpc))
In [5]: coords = ICRS([0.4, 0.3, 0.2] * u.km, representation_type=CartesianRepresentation)
In [6]: coords.transform_to(skyoffset_nodist).cartesian
Out[6]:
<CartesianRepresentation (x, y, z) in km
(0.52455168, 0.05113496, 0.11059271)>
In [7]: coords.transform_to(skyoffset_dist).cartesian
Out[7]:
<CartesianRepresentation (x, y, z) in km
(0.52455168, 0.05113496, 0.11059271)>
Activity
astrojuanlu commentedon Aug 25, 2018
And I guess the reason is here:
astropy/astropy/coordinates/builtin_frames/skyoffset.py
Lines 87 to 97 in b24b87f
Even if
originhas thedistanceattribute, it's not used. If this is not by design and it's a bug, I guess it shouldn't be very difficult to fix and I can do it myself.astrojuanlu commentedon Aug 25, 2018
...or maybe it is, because it changes the nature of the transformation to a
FunctionTransformorFunctionTransformWithFiniteDifference.adrn commentedon Oct 8, 2018
onceptually, transforming to a
SkyOffsetFrameis meant to remain at the same origin and just rotate to a coordinate system with the origin at the specified location. But I see how the docstring ofSkyOffsetFrameis vague enough that it sounds like it does what you need! Personally, I think the type of transformation you want should belong in a separate Frame class, and would still be useful. It sounds a bit more likeGalactocentric, which uses anAffineTransforminstead of aMatrixTransform.So, I would vote for turning this issue into 2:
SkyOffsetFrameto indicate that distance information in the origin is ignored, and not intended to be used.SkyOffsetandGalactocentric.Thoughts @eteq?
eteq commentedon Oct 24, 2018
I'm 👍 to @adrn's suggestion here. The
SkyOffsetFrameis oriented towards the "that's the way my telescope is pointed" use case, which implicitly means that the "distance" part of the origin shouldn't change. That's implicit in the "sky" part of the name but definitely needs to be cleared up in the docs!I'm going to rename this issue to be the second of these (since it's more useful to have @Juanlu001's sleuthing above in that issue), and make a separate one for the first/docstring suggestion.
[-]SkyOffsetFrame objects do not use distance information[/-][+]Implement a DistanceOffsetFrame that's like SkyOffsetFrame but includes distance effects[/+]eteq commentedon Oct 24, 2018
(Note that I'm not necessarily advocating
DistanceOffsetFrameas the name... not sure what the best name is. Just a "working title")eteq commentedon Oct 24, 2018
@Juanlu001 - I've not milestoned this v3.1 because I don't think @adrn or I have the time to work on this for feature freeze... but if you think you can do it in the next couple days I'd be happy to have it in! (But I realize that might be a big ask depending on how complex it turns out to be.)
astrojuanlu commentedon Oct 24, 2018
It's a bit short notice, so I prefer to leave it for the next release. Thanks @eteq!
bsipocz commentedon Apr 17, 2019
@Juanlu001 - I suppose you're not against removing the milestone here? (technically there are still 3 days until feature freeze, but I suspect you have enough on your plate already).
astrojuanlu commentedon Apr 17, 2019
Sure, let's postpone it! Thanks @bsipocz