-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Possible SkyCoord enhancements from cleaning up FRAME_ATTR_NAMES_SET behavior #5022
Copy link
Copy link
Open
Description
While investigating #5021 a couple other things about FRAME_ATTR_NAMES_SET came up. I'm noting them here so that someone (possibly me) can address them down the road.
The basic problem in #5021 is that the FRAME_ATTR_NAMES_SET function in sky_coordinate.py is dynamic, derived from the frame transform graph... But it's not particularly smart in how it does that. There are two specific optimizations I see that could be done fairly straightforwardly :
FRAME_ATTR_NAMES_SETshould be cached - it shouldn't need to be re-derived every time it's used (which is in lots of places inSkyCoord, including evendir). It's debatable where it should be cached - possibly on theSkyCoord(although that makes the second bullet below even worse), or perhaps the easiest is to cache it in theframe_transform_graph. In fact I think that might be the best way, because then theframe_transform_graphcan easily invalidate the cache when a frame or transform is added/removed. A third option would be to cache it in a global insky_coordinate.py, but then it's harder to see how it should be invalidated.- Attributes that were not set on
SkyCoordcreation shouldn't be stored/set in theSkyCoord- there's no reason to keep that there, and as more frames get added it starts to be quite a bit of the memory usage of aSkyCoord. Actually, with the changes in Fix SkyCoord AttributeError when a new frame is created after a SkyCoord has been inited #5021, I guess we could now just not set them at all and let the try/except clause added there send out the None that currently gets stored... but that might have some unintended consequence I've missed, so someone should maybe just try it and see.
Reactions are currently unavailable