You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Finalize decision on whether SkyCoord initializer should be bypassed for internal methods #7029
This is prompted by the work in #6941 . See the discussion there for more detail on this, but the basic question is: are we ok with the subtle change in #6941 where the SkyCoord initializer is bypassed for some of the internal replicate/copy/etc methods. @Cadair pointed out that SunPy overrides the __init__ to do various things, so we would potentially break such code this way.
Ideally, cleaning up the SkyCoord initializer will make this a moot point by making it so that the bypassing isn't needed. The backup plan is something like the items discussed in #6941 of a "make sanitized SkyCoord". This would then make the "bypass initializer" decision permanent.
IMO it's ok to bypass the SkyCoord initializer internally, but we shouldn't bypass the Frame initializer or it will break various SunPy magic. Also, this particular ship has sailed with #6941...
@adrn and I discussed this out-of-band and concluded that #6941 actually improves the situation because previously replicate/copy/etc went through the SkyCoord initializer rather than the __class__ initializer. Now you at least can potentially get the right kind of object out. And if someone wants to subclass SkyCoord and get all of these methods to work they really have to know what to do with apply anyway, so it's not a big of an extra hump to make them have to override this method.
Activity
adrn commentedon Oct 18, 2018
IMO it's ok to bypass the
SkyCoordinitializer internally, but we shouldn't bypass the Frame initializer or it will break various SunPy magic. Also, this particular ship has sailed with #6941...eteq commentedon Oct 18, 2018
@adrn and I discussed this out-of-band and concluded that #6941 actually improves the situation because previously replicate/copy/etc went through the
SkyCoordinitializer rather than the__class__initializer. Now you at least can potentially get the right kind of object out. And if someone wants to subclassSkyCoordand get all of these methods to work they really have to know what to do withapplyanyway, so it's not a big of an extra hump to make them have to override this method.On that basis, closing this issue.