I think this seems sensible. If I am understanding then the machinery to make this happen is all there because we keep track of whether an attribute was explicitly set or is just coming from the defaults. The relevant code is here:

            # Set the keyword args for making a new frame instance for the
            # transform.  If the supplied frame instance has a non-default
            # value set then use that, otherwise use the self attribute value
            # if it is not None.
            for attr in FRAME_ATTR_NAMES_SET():
                self_val = getattr(self, attr, None)
                frame_val = getattr(frame, attr, None)
                if (frame_val is not None and
                        attr not in frame._attr_names_with_defaults):
                    frame_kwargs[attr] = frame_val
                elif self_val is not None:
                    frame_kwargs[attr] = self_val

I haven't tested, but maybe in the final elif we can have another check of whether self_val is from a default (if self.is_frame_attr_default(attr):) then fall back to the frame_val. I haven't thought through the logic to see if there is a cleaner way to write this all.

BTW the check attr not in frame._attr_names_with_defaults) should also use the public method frame.is_frame_attr_default(attr) for better style.

I'm luck ... so far I've never had to write science code that uses FK4 ... not sure which default behaviour for it's equinox would be better / less error-prone.

Might be worth asking on the mailing list which equinox people expect (without showing what the current behaviour is).

I do think if you just make an FK5 coordinate and then transform to FK4 that most people would expect B1950 at the end.

EDIT: in the case where no equinox is ever explicitly specified.

I agree with @taldcroft and this is the behavior in e.g. kapteyn. The equinox for FK4 is always B1950 unless specified otherwise, and for FK5 it is always J2000 unless specified otherwise.

👍 , then.

I don't think this is supposed to be happening at all, so I think this is actually a bug rather than a "proposed change".

What's supposed to happen is that SkyCoord only "shadows" a frame attribute if it's explicitly specified at creation of the SkyCoord. So because you never explicitly specified the equinox, it's supposed to take whatever frame attribute is in the instance you passed. But that's not what's happening:

>>> FK4()
<FK4 Frame (equinox=B1950.000, obstime=B1950.000)>
>>> c.transform_to(FK4(equinox='B1950'))
<SkyCoord (FK4: equinox=B1950.000, obstime=J2000.000): ra=0.359230685957 deg, dec=0.721667458859 deg>

The default FK4 equinox is B1950, so it shouldn't be different to specify it given that the SkyCoord never specified it.

So if @taldcroft's suggestion resolves this, I'm 👍 for it.

Ok - it would be great if someone could implement a fix for this since I'm short on time in the next few days (but otherwise I'll get to it before 1.0)

Code attached. Based on @eteq's statement that this is a bug (I agree), I've put this for 0.4.3.

👍

This is passing tests now. Do you want to look at this @eteq ?

@eteq - how does d856c4b look?

@taldcroft - looks good! You might want to put backticks around "SkyCoord", but it's a bit ambiguous given it's referring to that thing itself, so whatever's you think is fine.

Other than that and my additional test suggestion above, I'm 👍 on merging. Thanks!