SkyCoord.replicate returns underlying BaseFrame type, not SkyCoord #15643

Activity

1. maxnoe

   added

   Bug

   on Nov 23, 2023
2. eerovaher

   added

   coordinates

   on Nov 23, 2023
3. eerovaher commented on Nov 23, 2023

   

   eerovaher

   on Nov 23, 2023

   Member

   More actions

   SkyCoord class has no replicate() method. SkyCoord instances expose the replicate() method of their underlying frame, just like how they expose many other methods and attributes.
4. mhvk commented on Nov 24, 2023

   

   mhvk

   on Nov 24, 2023

   Contributor

   More actions

   Hmm, that's pretty bad, SkyCoord should not have exposed that, or, arguably better, implement its own replicate.

   I guess generally one might expose all attributes, but probably should be more selective with methods.

   Aside: it is slightly unfortunate that astropy has replicate and copy instead of copy and deepcopy, resp., but I fear that ship has sailed.
5. nstarman commented on Nov 24, 2023

   

   nstarman

   on Nov 24, 2023

   Member

   More actions

   Aside: it is slightly unfortunate that astropy has replicate and copy instead of copy and deepcopy

   We also have clone!

   but I fear that ship has sailed.

   When Cosmology becomes a dataclass I'm planning to deprecate the .clone method in favor of replace, which is the dataclasses library standard and suggestion. The .clone method will just be a deprecated alias of replace that'll stick around for a good long while.

   We can do the same for all the other miscellaneous implementations!

   🎉React with 🎉1Reacted by Clément Robert
6. ayshih commented on Nov 24, 2023

   

   ayshih

   on Nov 24, 2023

   Contributor

   More actions

   Hmm, that's pretty bad, SkyCoord should not have exposed that, or, arguably better, implement its own replicate.

   SkyCoord having its own replicate() makes a lot of sense, since otherwise there's no way to perserve attributes that are not used for the particular frame. I'll note that it creates a bit of non-parallelism if replicate_without_data() is still exposed, because the output of that method cannot be a SkyCoord. (That is, it's unfortunate the name of that method didn't happen to be something like replicate_only_frame().)

   I guess generally one might expose all attributes, but probably should be more selective with methods.

   I'll point out that sunpy has multiple frame-specific methods and properties (beyond the frame attributes) that are defined on frame subclasses, and it's convenient that they are automatically exposed as methods and properties of corresponding SkyCoord instances. It wouldn't be a huge deal to educate users to call coord.frame.whatever() instead of coord.whatever(), but some internals might need to do more SkyCoord/not-SkyCoord checking. It may also have users thinking more about the implementation than they need to be.

   Aside: it is slightly unfortunate that astropy has replicate and copy instead of copy and deepcopy, resp., but I fear that ship has sailed.

   I imagine that the vast majority of calls to replicate() are to change the value of at least one frame attribute. As such, copy()/deepcopy() are not great analogies, but as mentioned by @nstarman, the dataclasses replace() would be.
7. nstarman commented on Dec 4, 2024

   

   nstarman

   on Dec 4, 2024

   Member

   More actions

   Aside: it is slightly unfortunate that astropy has replicate and copy instead of copy and deepcopy, resp., but I fear that ship has sailed.

   #16887!