Add Geodetic Representations #40
Conversation
FYI it is more than just that. We need asdf-astropy support to complete the transition to the 1.1.0 |
| - A SphericalRepresentation | ||
| - | | ||
| !<tag:astropy.org:astropy/coordinates/representation-1.1.0> | ||
| components: | ||
| distance: !unit/quantity-1.1.0 {unit: !unit/unit-1.0.0 AU, value: 1.0} | ||
| lat: !<tag:astropy.org:astropy/coordinates/latitude-1.0.0> {unit: !unit/unit-1.0.0 deg, | ||
| value: 10.0} | ||
| lon: !<tag:astropy.org:astropy/coordinates/longitude-1.0.0> | ||
| unit: !unit/unit-1.0.0 deg | ||
| value: 10.0 | ||
| wrap_angle: !<tag:astropy.org:astropy/coordinates/angle-1.0.0> {unit: !unit/unit-1.0.0 deg, | ||
| value: 360.0} | ||
| type: SphericalRepresentation | ||
| - | ||
| - A CartesianDifferential | ||
| - | | ||
| !<tag:astropy.org:astropy/coordinates/representation-1.1.0> | ||
| components: | ||
| d_x: !unit/quantity-1.1.0 {unit: !unit/unit-1.0.0 km s-1, value: 100.0} | ||
| d_y: !unit/quantity-1.1.0 {unit: !unit/unit-1.0.0 km s-1, value: 200.0} | ||
| d_z: !unit/quantity-1.1.0 {unit: !unit/unit-1.0.0 km s-1, value: 3141.0} | ||
| type: CartesianDifferential |
There was a problem hiding this comment.
asdf-astropy does not currently support the 1.1.0 tag meaning this will fall back on astropy which fails due to warnings.
There was a problem hiding this comment.
Looking into these geodetic classes it appears that the changes and API for this are not yet finalized (based off this open issue: astropy/astropy#11170).
The original PR adding these representations involved a discussion about deprecating EarthLocation.to_geodetic and making a new method that returns these new types:
astropy/astropy#11086
However it was decided to leave these types as internal and keep the currently used astropy.coordinates.earth.GeodeticLocation that is produced when calling astropy.coordinates.earth.EarthLocation.to_geodetic.
It appears that the return type for this is unsupported in asdf-astropy on main and with this PR.
import asdf
import astropy.coordinates.earth
c = astropy.coordinates.earth.EarthLocation.of_site('Keck Observatory')
g = c.to_geodetic()
asdf.AsdfFile({'g': g})fails with
Warning: Failed to serialize instance of <class 'astropy.coordinates.earth.GeodeticLocation'>, converting to list instead
warnings.warn(f"Failed to serialize instance of {type(node)}, converting to list instead", AsdfWarning)
I'm inclined to wait until the new geodetic api is finished before adding support for it in the asdf schema.
I will close this PR since you feel this way. I want to point out that these representations have been added to the public API for astropy: https://github.com/astropy/astropy/blob/08dba88089f124064492d3fde40919561640ea51/astropy/coordinates/representation/__init__.py#L9-L14 and that your API comments are irrelevant since we only care about the data contained within the object not its general API with astropy. Our support for astropy representations is based on the baseline object and the common way they store their information: https://github.com/astropy/asdf-astropy/blob/6c0e91a1db88d1c7253ac476dcc5eebc2a1a33fd/asdf_astropy/converters/coordinates/representation.py#L24-L39 not some other astropy API. |
fixes #39