Skip to content

Removal of OrderedDescriptor has broken SkyOffsetFrame when using SkyCoord #11254

Closed

Description

@ayshih

The removal of OrderedDescriptor (#11099, merged today) has broken SkyOffsetFrame when using SkyCoord:

>>> import astropy.units as u
>>> from astropy.coordinates import SkyCoord, SkyOffsetFrame, ICRS

>>> origin = SkyCoord(10*u.deg, 0*u.deg, frame='icrs')
>>> offset_frame = SkyOffsetFrame(origin=origin)
>>> coord = SkyCoord(3*u.deg, 4*u.deg, frame=offset_frame)
>>> coord
<SkyCoord (SkyOffsetICRS: rotation=0.0 deg, origin=<ICRS Coordinate: (ra, dec) in deg
    (10., 0.)>): (lon, lat) in deg
    (3., 4.)>

>>> coord.transform_to('icrs')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-21-d349054966e0> in <module>
----> 1 coord.transform_to('icrs')

c:\users\ayshih\onedrive\code\astropy\astropy\coordinates\sky_coordinate.py in transform_to(self, frame, merge_attributes)
    625             frame_kwargs.pop(attr)
    626 
--> 627         return self.__class__(new_coord, **frame_kwargs)
    628 
    629     def apply_space_motion(self, new_obstime=None, dt=None):

c:\users\ayshih\onedrive\code\astropy\astropy\coordinates\sky_coordinate.py in __init__(self, copy, *args, **kwargs)
    321             for attr in skycoord_kwargs:
    322                 # Setting it will also validate it.
--> 323                 setattr(self, attr, skycoord_kwargs[attr])
    324 
    325             if info is not None:

c:\users\ayshih\onedrive\code\astropy\astropy\coordinates\sky_coordinate.py in __setattr__(self, attr, val)
    809             super().__setattr__('_' + attr, val)
    810             # Validate it
--> 811             frame_transform_graph.frame_attributes[attr].__get__(self)
    812             # And add to set of extra attributes
    813             self._extra_frameattr_names |= {attr}

c:\users\ayshih\onedrive\code\astropy\astropy\coordinates\attributes.py in __get__(self, instance, frame_cls)
    102                 out = getattr(instance, self.secondary_attribute, self.default)
    103 
--> 104         out, converted = self.convert_input(out)
    105         if instance is not None:
    106             instance_shape = getattr(instance, 'shape', None)  # None if instance (frame) has no data!

c:\users\ayshih\onedrive\code\astropy\astropy\coordinates\attributes.py in convert_input(self, value)
    445         if value is None:
    446             return None, False
--> 447         elif isinstance(value, self._frame):
    448             return value, False
    449         else:

TypeError: isinstance() arg 2 must be a type or tuple of types

Using frames directly instead of SkyCoord works fine:

>>> coord.frame.transform_to(ICRS())
<ICRS Coordinate: (ra, dec) in deg
    (13., 4.)>

It looks like it's related to the additional processing that SkyCoord does for frame attributes, and the fact that this line:

members['origin'] = CoordinateAttribute(frame=framecls, default=None)

is no longer working as intended (it's supposed to overwrite the created frame's origin frame attribute from one with frame=None to one with frame=framecls). The created frame's origin frame attribute doesn't have frame=framecls:

>>> offset_frame.frame_attributes['origin']._frame is None
True

Activity

  1. added
    🔥Critical
    Affects-devPRs and issues that do not impact an existing Astropy release
    on Jan 17, 2021
  2. added this to the v4.3 milestone on Jan 17, 2021
  3. pllim commented on Jan 17, 2021

    @pllim
    Member

    Oh noes! @adrn or @mhvk , can you please advise?

    I am guessing that this will block 4.3 release unless we back out that part of the code that was changed in #11099.

  4. mhvk commented on Jan 17, 2021

    @mhvk
    Contributor

    Ouch! Probably fairly easy to fix... cc @embray as well.

  5. mhvk commented on Jan 17, 2021

    @mhvk
    Contributor

    p.s. How the heck is this not tested!!!

  6. mhvk commented on Jan 17, 2021

    @mhvk
    Contributor

    Trying to produce a fix... (thanks, @ayshih, for doing such detailed detective work...)

  7. mhvk commented on Jan 17, 2021

    @mhvk
    Contributor

    That was a bit more tricky than I thought but it did end up with a fairly clean solution. See #11255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    🔥CriticalAffects-devPRs and issues that do not impact an existing Astropy releaseBugcoordinates

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions