Frame attributes are retained, so further transforms (like back to TelescopeFrame) work

mmh. You give an instance of a Frame that has no focal length. Frames are immutable, so I don't really see a way how to fix that for this specific example.

This would work:

new_geom = geom.transform_to(EngineeringCameraFrame(focal_length=geom.frame.focal_length))

but only if the current frame as a focal_length attribute.

astropy also allows using frame classes not instances in transform_to in some cases (mostly the once that do not actually need attributes I think), which would enable us to set the attributes, but the current CameraGeometry.transform_to method only works with frame instances:

In [7]: geom.transform_to(EngineeringCameraFrame)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[7], line 1
----> 1 geom.transform_to(EngineeringCameraFrame)

File ~/Uni/CTA/ctapipe/ctapipe/instrument/camera/geometry.py:257, in CameraGeometry.transform_to(self, frame)
    254 points_x = getattr(points, x_name)
    255 points_y = getattr(points, y_name)
--> 257 trans_x_name, trans_y_name = get_representation_component_names(frame)
    258 points_trans_x = getattr(points_trans, trans_x_name)
    259 points_trans_y = getattr(points_trans, trans_y_name)

File ~/Uni/CTA/ctapipe/ctapipe/coordinates/__init__.py:47, in get_representation_component_names(frame)
     45 def get_representation_component_names(frame):
     46     """Return the component names of a Frame (or SkyCoord)"""
---> 47     return tuple(frame.get_representation_component_names().keys())

TypeError: get_representation_component_names() missing 1 required positional argument: 'self'

Yes, I guess frames don't work in the way I would like - it's a bit annoying to have to pass all frame attributes to a new Frame that has the same ones, but maybe there is no option. SOmething like:

camframe = CameraFrame(focal_length=12*u.m, obstime=Time.now())
newframe = EngineeringCameraFrame(camframe)

# or even

newframe = EngineeringCameraFrame(**camframe.frame_attributes)

where any common attributes are copied would be nice. Or maybe there is a canonical way to do this that I am missing? Do you really have to set all attributes? (for CameraFrame and EngineeringFrame there are already 5 attributes, so it can get annoying).

For example, when I do this, the frame attributes are retained, so why are they not in the CameraFrame case?

c1 = SkyCoord(ra=12*u.h, dec=10*u.deg, frame="icrs", obstime=t.Time.now())
c2 = c1.transform_to("fk4")
print(c2)

<SkyCoord (FK4: equinox=B1950.000, obstime=2023-03-02 17:54:23.899980): (ra, dec) in deg
    (179.35902946, 10.27828609)>

c2 still has an obstime, it was not reset to 0

Ah, yes, I see it's the difference between passing an instance and a class... So perhaps it is fixable in CameraGeometry. It does seem like the "correct" way is to pass classes not instances, at least it allows attributes to be assigned in the SkyCoord class

c3 = SkyCoord(x=11*u.m, y=0*u.m, obstime=t.Time.now(), focal_length=14*u.m, frame=CameraFrame)
c3.transform_to(EngineeringCameraFrame)

<SkyCoord (CameraFrame: focal_length=14.0 m, rotation=0.0 rad, telescope_pointing=None, obstime=2023-03-02 18:02:46.329504, location=None): (x, y) in m
    (11., 0.)>

@kosack And the fact that SkyCoord retains any frame attributes and attaches them if the current frame also has these attributes. That's why the high-level SkyCoord attribute is preferred over frame instances.

So maybe a fix would also be to have pix_x and pix_y be properties into a coord: SkyCoord instance

The current fix I made was just to support passing frame classes,

But I like the idea of using a SkyCoord internally (and even exposing it). The previous geom.pix_x and geom.pix_y would just be properties to maintain backward compatibility, and geom.pix_coord would be a SkyCoord instance. That would solve my other issue that the TelescopeFrame doesn't have a focal_length, so it gets lost. If SkyCoord retains the full set, that that is no longer an issue. I would then also remove the geom.frame attribute, as it would be in the SkyCoord instance. Will have to change a bit how that gets created/set, but I think it is nicer.