Skip to content

Latest commit

 

History

History
464 lines (384 loc) · 18.6 KB

File metadata and controls

464 lines (384 loc) · 18.6 KB

Removing data storage (representations) from coordinate frames

Authors (alphabetical): Jeff Jennings, Adrian Price-Whelan, Nathaniel Starkman, Marten van Kerkwijk

date-created:2024 11 04
date-last-revised:2026 05 27
date-accepted:2026 05 27
type:Standard Track
status:Accepted

Abstract

Following the rationale presented in APE 5, coordinate frames in astropy.coordinates currently store metadata used to construct the frame (i.e., for transforming between frames) and may also store coordinate data itself. This duplicates functionality with SkyCoord, which acts as a container for both coordinate data and reference frame information. We propose to change the frame classes such that they only store metadata and never coordinate data, superseding this aspect of the implementation in APE 5. This would make the implementation more modular and performant, remove ambiguity for users from having nearly duplicate functionality with slightly different APIs, and better satisfy the principle of Separation of Concerns.

Detailed description

The coordinate frame classes (subclasses of BaseCoordinateFrame, e.g., ICRS, Galactic, FK4, etc.) are used to represent astronomical reference frames. These may also contain position, velocity, time, or other information about the reference frame relative to another reference frame, which is used to transform coordinates between the reference frames that exist in the astropy.coordinates ecosystem. For example, the AltAz frame class, which is used when referencing sky coordinates in altitude and azimuth, must contain a location on Earth and a time in order to transform to and from an inertial reference frame like the ICRS. In addition to reference frame information, these frame classes can also contain coordinate data (positions, velocities, or other differentials), which are stored internally using the BaseRepresentation and BaseDifferential subclasses.

The above reflects a design choice in the coordinate frame implementation in APE 5 that, following several years of use, we have learned from and think best to revise. We now view it as an insufficient separation of concerns. The coordinate frame class deals with two separate issues: the definition of the frame and the storage of coordinate data within that frame. Ideally the code would be more modularly structured, where the coordinate frame class only defines the reference frame. This was considered in APE 5 (discussed as one alternative to the implementation pursued there), but it was deemed too difficult to implement the 'high-level' classes as generics. With the proposed framework in this APE, we no longer find such a difficulty; the coordinate frame class only defines the reference frame, and it becomes the purview of another class – like SkyCoord or the newly-proposed Coordinate – to bring these concerns together and to represent data (using BaseRepresentation and BaseDifferential subclasses) in a given reference frame (using a CoordinateFrame class). As a demonstration of the current state of duplicated functionality, these two initializations effectively represent the same thing, but return different objects with similar but not identical APIs:

c1 = SkyCoord(1., 2., frame="icrs", units="deg")
c2 = ICRS(1. * u.deg, 2. * u.deg)

Consequently, as one simple example, user confusion can arise from an operation as common as transforming reference frames:

c1g = c1.transform_to(Galactic())  # Works
c2g = c2.transform_to(Galactic())  # Works
c1g = c1.transform_to("galactic")  # Works
c2g = c2.transform_to("galactic")  # Raises ConvertError

We suggest the situation should instead be analogous to the structure implemented for astropy.units: the Unit class is able to transform between units, but it is not concerned with how the associated values are stored (which is instead handled by Quantity). Translating to coordinates, units are like CoordinateFrame, the values are the coordinate data (BaseRepresentation), and Quantity is like SkyCoord.

Having both the frame classes as well as SkyCoord be able to store and handle data has resulted in a few notable types of issues. First, it has results in a large amount of code duplication. Second, the reliance on SkyCoord.__getattr__() has led to at least one method that SkyCoord implements accidentally (astropy/astropy#15643), and there may be more. Third, having both the frame classes and SkyCoord has also required duplicated or even quadrupled tests, in order to test both BaseCoordinateFrame and SkyCoord methods with both BaseCoordinateFrame and SkyCoord arguments - where the tests have not covered every combination, problems have gone unnoticed. Restructuring the frame classes to remove data storage will allow for much more maintainable, de-duplicated code. It will also make it easier to contribute: if there is a problem one would like to solve in a given method, if one looks in SkyCoord, one will likely find that it does not exist, and might struggle to find that instead it is defined on BaseCoordinateFrame and gets dynamically called via SkyCoord.__getattr__. Indeed, the construction of BaseCoordinateFrame ends up complicating SkyCoord, which has to manage the coordinate data through the stored BaseCoordinateFrame.

Another issue with the current implementation of coordinate frames is that the optional inclusion of coordinate data makes the reference frames “multi-modal”. This creates different usage modes (with and without data), each exhibiting different behavior. For instance, some methods such as separation work for coordinate-frame instances that contain data, but they lead to faults for instances without data. While this multi-modal structure as motivated in APE 5 can be seen as a benefit for interpretability of the logic, we also now see it from the perspective of an anti-pattern. It forces any code interacting with the frame classes to handle both cases (checking frame.has_data), complicating the codebase. Moreover static analyzers cannot determine which case is being used; with separated frames and data, static analyzers will be able to prevent this entire class of errors. We thus find it worthwhile to separate coordinate data from reference frames at the possible expense of developers having to learn this new framework.

The major points discussed thus far – separation of concerns and code duplication – concern maintainers. However user experience is the more important consideration. In this arena too, separating frames from data storage has its advantages. Perhaps most importantly, documentation will be more obvious: the methods and attributes are defined on SkyCoord (and Coordinate) proper, so Sphinx will know how to typeset those, while type checkers can help users in finding and using them properly. It will also be easier: following the Zen of Python, "There should be one-- and preferably only one --obvious way to do it." The present overlap leads to confusion wherein beginner users end up creating BaseCoordinateFrame instances, when the docs are clear that these are for more advanced users and that SkyCoord is to be preferred. The system will also be less fragile; with these proposed changes, users - and importantly downstream developers who subclass SkyCoord - will have a more clear, introspectable, and robust system.

Finished Product

The end result of the implementation of this APE will be two separate hierarchies of classes: reference frame classes and coordinate classes which bring together a reference frame and coordinate data. We discuss each class type in turn.

Reference frame classes only hold information pertaining to the reference frame they represent and never actual coordinate data in that reference frame. This is consistent with our mathematical framework, as the reference frame mediates how coordinate data is understood (e.g., distance measures) or interacts (e.g., separation from other coordinates), but the coordinate data itself is actually independent of that information.

Classes like SkyCoord will be composed structures bringing together the reference frame (an instance of a BaseFrame subclass) and the coordinate data (BaseRepresentation objects). We also introduce a new class, Coordinate, which is akin to SkyCoord (containing both frame and data), but without extra features like keeping frame attributes not associated with the current frame, caching and flexible input parsing. In this way Coordinate operates very similarly to the current BaseCoordinateFrame objects when they have data, and is meant to be their direct replacement in the new framework as well as a more lightweight and performant alternative to SkyCoord.

We illustrate the new framework with the following pseudocode.

class BaseFrame:
    ...

class ICRSFrame(BaseFrame):
    pass  # no frame attributes

class FK5Frame(BaseFrame):
    equinox: TimeAttribute

# ------

class BaseCoordinate:
    frame: BaseFrame
    data: BaseRepresentation

class Coordinate(BaseCoordinate):
    ...  # it's fast.

class SkyCoord(BaseCoordinate):

    def __init__(...):  # flexible input parsing
        ...

Branches and pull requests

No direct progress on these changes has yet occurred. Discussion of these ideas has however arisen in multiple issues and pull requests, demonstrating the need for and utility of the proposed changes.

Several issues have been raised regarding topics such as confusion differentiating the use of frame and SkyCoord for data storage, and problems arising in other astropy subpackages when using frames that store data. For example:

Additionally, multiple pull requests have factored out common code between frames and SkyCoord, showing that there is no proper separation of concern:

Further, pull requests have added methods to make frames and SkyCoord even more similar, underscoring that frames with data should not be separate entities from SkyCoord:

In addition, many of these ideas have been developed and tested in parallal in the JAX-oriented library coordinax. Many of the developers of that library are also active Astropy developers and the development effort towards coordinax informs, tests, and validates the ideas presented in this APE. In short, it works.

Implementation

The direct use of coordinate frames instead of SkyCoord is common. In particular ICRS objects are frequently created with data. Given the prevalent use, it is imperative to maintain backward compatibility and not break the API too quickly. Therefore, we propose implementing this APE through the 4 steps (and substeps) below. See the Usage patterns section for practical examples of how to use the new framework.

  1. Splitting the frame classes into two hierarchies: ones with and without data, with the data-less ones getting new names.
  2. Adding a new Coordinate class that is similar to SkyCoord, but which does not keep any frame attributes not in the current frame, and does not have extra features like caching and flexible input parsing. It will only accept data-less frame classes.
  3. Switching SkyCoord to use the data-less frame classes, and enabling automatic conversion of the with-data frames into SkyCoord objects.
  4. Deprecating the legacy with-data frame classes, and eventually removing them after a deprecation period that adheres to APE 2.
    • Emitting warnings when instantiated.
    • Still warn, but return a Coordinate, not an instance of its class type (by overriding __new__). If there are justified objections to overriding __new__, an alternative would be to prolong the deprecation period.
    • Remove.

The fourth step is illustrated in the following pseudocode:

# === Reference Frame (no data) ===

class BaseFrame:
    ...

    # Like unit.to(new_unit, value)
    def transform_data_to(self, frame: BaseFrame, data: BaseRepresentation) -> BaseRepresentation:
        """Used by BaseCoordinate for transformation."""
        ...

class ICRSFrame(BaseFrame):
    pass  # no frame attributes

class FK5Frame(BaseFrame):
    equinox: TimeAttribute

# === Coordinates (data + frame) ===

class BaseCoordinate:
    """Base class for data in a reference frame."""
    frame: BaseFrame
    data: BaseRepresentation
    ...

class SkyCoord(BaseCoordinate):
     """Data in a reference frame, batteries included."""

    def __init__(...):  # flexible input parsing
        # If the frame is a LegacyBaseCoordinateFrame then it is
        # split into a BaseFrame and BaseRepresentation.
        ...

    _cache: dict[str, Any]  # cache

class Coordinate(BaseCoordinate):
    """Data in a reference frame."""
    ...  # Direct and fast.

# === Legacy Coordinate Classes ===

class BaseCoordinateFrame(BaseCoordinate):
    """Reference frames (with optional data storage)."""

    def __new__(self):
        warnings.warn("Please use SkyCoord")

    @abstractpropery # implemented on subclasses
    def frame(self) -> BaseFrame:
        ...

class ICRS(BaseCoordinateFrame, ICRSFrame):
    ...

class FK5(BaseCoordinateFrame, FK5Frame):
    ...

We also provide a fuller prototype <https://github.com/jeffjennings/ape26_scratch/blob/main/ape26_prototype.py> and a demo notebook <https://github.com/jeffjennings/ape26_scratch/blob/main/demo.ipynb> implementing these changes.

Usage patterns

The following pseudocode presents typical use cases of the new framework at each of the four steps outlined in the previous section.

# === Step 1 ===

# Unchanging API
# Normal type hierarchy
ra, dec = ..., ...
rep = SphericalRepresentation(ra, dec)
c = ICRS(rep)  # the same. not yet deprecated.
sc = SkyCoord(c)
# Flexible inputs
c = ICRS(ra_arr, dec_arr)  # the same. not yet deprecated.
c = ICRS(rep)  # the same. not yet deprecated.
sc = SkyCoord(ra_arr, dec_arr, frame=ICRS())  # the same. not yet deprecated.
sc = SkyCoord(rep, frame=ICRS())  # the same. not yet deprecated.

# New API
frame = ICRSFrame()
sc = SkyCoord(rep, frame=frame)
sc = SkyCoord(ra_arr, dec_arr, frame=frame)

# === Step 2 ===

# Unchanging API
# Normal type hierarchy
ra, dec = ..., ...
rep = SphericalRepresentation(ra, dec)
c = ICRS(rep)  # the same. not yet deprecated.
sc = SkyCoord(c)
# Flexible inputs
c = ICRS(ra_arr, dec_arr)  # the same. not yet deprecated.
c = ICRS(rep)  # the same. not yet deprecated.
sc = SkyCoord(ra_arr, dec_arr, frame=ICRS())  # the same. not yet deprecated.
sc = SkyCoord(rep, frame=ICRS())  # the same. not yet deprecated.

# New API
frame = ICRSFrame()
c = Coordinate(rep, frame)
sc = SkyCoord(rep, frame=frame)
sc = SkyCoord(c.data, c.frame)  # (just showing the strict constructor)
sc = SkyCoord(c)  # flexible
sc = SkyCoord(ra_arr, dec_arr, frame=frame)  # flexible

# === Step 3 ===
# Unchanging API
# Normal type hierarchy
ra, dec = ..., ...
rep = SphericalRepresentation(ra, dec)
c = ICRS(rep)  # the same. not yet deprecated.
sc = SkyCoord(c)
# Flexible inputs
c = ICRS(ra_arr, dec_arr)  # the same. not yet deprecated.
c = ICRS(rep)  # the same. not yet deprecated.
sc = SkyCoord(ra_arr, dec_arr, frame=ICRS())  # redirects to SkyCoord(ra_arr, dec_arr, frame=ICRSFrame())
sc = SkyCoord(rep, frame=ICRS())  # redirects to SkyCoord(ra_arr, dec_arr, frame=ICRSFrame())

# New API
frame = ICRSFrame()
c = Coordinate(rep, frame)
sc = SkyCoord(rep, frame=frame)
sc = SkyCoord(c)  # flexible
sc = SkyCoord(ra_arr, dec_arr, frame=frame)  # flexible

# === Step 4 ===
# Unchanging API
# Normal type hierarchy
ra, dec = ..., ...
rep = SphericalRepresentation(ra, dec)

# Deprecated API
c = ICRS(rep)
c = ICRS(ra_arr, dec_arr)
sc = SkyCoord(c)
sc = SkyCoord(ra_arr, dec_arr, frame=ICRS())  # ICRS -> ICRSFrame
sc = SkyCoord(rep, frame=ICRS())  # ICRS -> ICRSFrame

# New API
frame = ICRSFrame()
c = Coordinate(rep, frame)
sc = SkyCoord(rep, frame=frame)
sc = SkyCoord(c)  # flexible
sc = SkyCoord(ra_arr, dec_arr, frame=frame)  # flexible

Decision rationale

The Coordination Committee is accepting this APE under the provisions that (1) the work can be completed within approximately the next year --- roughly by Q2 2027 --- and (2) the authors provide clear transition documentation that will enable users to update their code to the new coding patterns before the old patterns are fully deprecated. Given that much of the coding work is already complete, our primary concern is the follow-on effect on users.