Repository: astropy/astropy

Issue:
Declination column missing when writing tables with SkyCoord columns to ECSV
### Description

If you create a new SkyCoord instance from an existing one, passing the `representation_type=` kwarg to the constructor, add it to a table, and write the table to ECSV, columns are missing.

For example, this code:

```python
coords = SkyCoord(np.zeros(5), np.ones(5), np.zeros(5), representation_type=CartesianRepresentation)
coords = SkyCoord(coords, representation_type=UnitSphericalRepresentation)
Table({"coords": coords}).write(sys.stdout, format="ascii.ecsv")
```

results in the following table output, missing the declination column:

```
# %ECSV 1.0
# ---
# datatype:
# - {name: coords.ra, unit: deg, datatype: float64}
# meta: !!omap
# - __serialized_columns__:
#     coords:
#       __class__: astropy.coordinates.sky_coordinate.SkyCoord
#       frame: icrs
#       ra: !astropy.table.SerializedColumn
#         __class__: astropy.coordinates.angles.core.Longitude
#         unit: &id001 !astropy.units.Unit {unit: deg}
#         value: !astropy.table.SerializedColumn {name: coords.ra}
#         wrap_angle: !astropy.coordinates.Angle
#           unit: *id001
#           value: 360.0
#       representation_type: unitspherical
# schema: astropy-2.0
coords.ra
90.0
90.0
90.0
90.0
90.0
```

### Expected behavior

Each of the three examples in the code below should print:

```
# %ECSV 1.0
# ---
# datatype:
# - {name: coords.ra, unit: deg, datatype: float64}
# - {name: coords.dec, unit: deg, datatype: float64}
# meta: !!omap
# - __serialized_columns__:
#     coords:
#       __class__: astropy.coordinates.sky_coordinate.SkyCoord
#       dec: !astropy.table.SerializedColumn
#         __class__: astropy.coordinates.angles.core.Latitude
#         unit: &id001 !astropy.units.Unit {unit: deg}
#         value: !astropy.table.SerializedColumn {name: coords.dec}
#       frame: icrs
#       ra: !astropy.table.SerializedColumn
#         __class__: astropy.coordinates.angles.core.Longitude
#         unit: *id001
#         value: !astropy.table.SerializedColumn {name: coords.ra}
#         wrap_angle: !astropy.coordinates.Angle
#           unit: *id001
#           value: 360.0
#       representation_type: spherical
# schema: astropy-2.0
coords.ra coords.dec
90.0 0.0
90.0 0.0
90.0 0.0
90.0 0.0
90.0 0.0
```

However, only the first two tables print that. The last table is missing the declination column.

### How to Reproduce

```python
from astropy import units as u
from astropy.coordinates import CartesianRepresentation, UnitSphericalRepresentation, SkyCoord
import numpy as np
from astropy.table import Table
import sys

coords = SkyCoord(np.full(5, 90), np.zeros(5), unit=u.deg)
Table({"coords": coords}).write(sys.stdout, format="ascii.ecsv")

coords = SkyCoord(np.zeros(5), np.ones(5), np.zeros(5), representation_type=CartesianRepresentation)
coords.representation_type = UnitSphericalRepresentation
Table({"coords": coords}).write(sys.stdout, format="ascii.ecsv")

coords = SkyCoord(np.zeros(5), np.ones(5), np.zeros(5), representation_type=CartesianRepresentation)
coords = SkyCoord(coords, representation_type=UnitSphericalRepresentation)
Table({"coords": coords}).write(sys.stdout, format="ascii.ecsv")
```

### Versions

```
Python 3.11.10 (main, Sep  7 2024, 05:47:53) [Clang 15.0.0 (clang-1500.1.0.2.5)]
astropy 6.1.3
Numpy 2.1.1
pyerfa 2.0.1.4
Scipy 1.14.1
Matplotlib 3.9.2
```


Discussion:
github-actions[bot]: Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  - [ ] Do the proposed changes actually accomplish desired goals?
  - [ ] Do the proposed changes follow the [Astropy coding guidelines](https://docs.astropy.org/en/latest/development/codeguide.html)?
  - [ ] Are tests added/updated as required? If so, do they follow the [Astropy testing guidelines](https://docs.astropy.org/en/latest/development/testguide.html)?
  - [ ] Are docs added/updated as required? If so, do they follow the [Astropy documentation guidelines](https://docs.astropy.org/en/latest/development/docguide.html)?
  - [ ] Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for [rebase](https://docs.astropy.org/en/latest/development/development_details.html#rebase-if-necessary) and [squash](https://docs.astropy.org/en/latest/development/development_details.html#squash-if-necessary).
  - [ ] Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the [bot](https://docs.astropy.org/en/latest/development/development_details.html#pre-commit).
  - [ ] Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  - [ ] Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  - [ ] At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) *before* merge.
pllim: @mhvk , you milestoned to v7 but also added backport label. They are in conflict. Which one is correct? Hope you can clarify. Thanks!
mhvk: Oops, thanks for catching that. I thought it would be good to backport!
mhvk: For further review: the last commit only affected a comment, so I used `[skip ci]` - the previous commit passed all tests, so this should be fine. Do squash the two on merge!
mhvk: I thought it really made most sense to test the table writing by adding a column to `mixin_columns`. Beautifully, that also brought out an extra bug, which is now fixed. I also used that opportunity to follow your suggestion of having two separate commits, one adding the tests, and the other adding the fixes. Much better now, thanks for the review!

Patch:
diff --git a/astropy/coordinates/baseframe.py b/astropy/coordinates/baseframe.py
index bbef224ab3..86e9d4cb37 100644
--- a/astropy/coordinates/baseframe.py
+++ b/astropy/coordinates/baseframe.py
@@ -184,31 +184,46 @@ class CoordinateFrameInfo(MixinInfo):
     def _represent_as_dict(self):
         coord = self._parent
         attrs = []
+        representation_type = coord.representation_type
+        differential_type = None
         if coord.has_data:
             attrs.extend(coord.representation_component_names)
             # Don't output distance unless it's actually distance.
-            if isinstance(coord.data, r.UnitSphericalRepresentation):
+            if issubclass(
+                representation_type, r.SphericalRepresentation
+            ) and isinstance(coord.data, r.UnitSphericalRepresentation):
                 attrs = attrs[:-1]
 
             diff = coord.data.differentials.get("s")
             if diff is not None:
+                differential_type = coord.differential_type
                 diff_attrs = list(coord.get_representation_component_names("s"))
-                # Don't output proper motions if they haven't been specified.
-                if isinstance(diff, r.RadialDifferential):
-                    diff_attrs = diff_attrs[2:]
-                    # Don't output radial velocity unless it's actually velocity.
-                elif isinstance(
-                    diff,
-                    (r.UnitSphericalDifferential, r.UnitSphericalCosLatDifferential),
+                if issubclass(
+                    differential_type,
+                    (r.SphericalDifferential, r.SphericalCosLatDifferential),
                 ):
-                    diff_attrs = diff_attrs[:-1]
+                    # Don't output proper motions or radial velocities unless
+                    # they've been specifically passed in.
+                    if isinstance(diff, r.RadialDifferential):
+                        diff_attrs = diff_attrs[2:]
+                    elif isinstance(
+                        diff,
+                        (
+                            r.UnitSphericalDifferential,
+                            r.UnitSphericalCosLatDifferential,
+                        ),
+                    ):
+                        diff_attrs = diff_attrs[:-1]
                 attrs.extend(diff_attrs)
 
         attrs.extend(frame_transform_graph.frame_attributes.keys())
 
         out = super()._represent_as_dict(attrs)
 
-        out["representation_type"] = coord.representation_type.get_name()
+        out["representation_type"] = representation_type.get_name()
+        if differential_type is not None:
+            out["differential_type"] = differential_type.get_name()
+
         # Note that coord.info.unit is a fake composite unit (e.g. 'deg,deg,None'
         # or None,None,m) and is not stored. The individual attributes have
         # units.
@@ -843,6 +858,10 @@ class BaseCoordinateFrame(ShapedLikeNDArray):
                 RepresentationMapping("d_distance", "radial_velocity", lin_v_unit),
             ],
         )
+        repr_info.setdefault(
+            r.RadialDifferential,
+            [RepresentationMapping("d_distance", "radial_velocity", lin_v_unit)],
+        )
         repr_info.setdefault(
             r.CartesianDifferential,
             [RepresentationMapping(f"d_{c}", f"v_{c}", lin_v_unit) for c in "xyz"],
diff --git a/docs/changes/coordinates/16999.bugfix.rst b/docs/changes/coordinates/16999.bugfix.rst
new file mode 100644
index 0000000000..79ef887b92
--- /dev/null
+++ b/docs/changes/coordinates/16999.bugfix.rst
@@ -0,0 +1,5 @@
+Avoid some components not being included in table output of coordinates if
+the representation type was ``"unitspherical"``.
+
+In the process, also ensured that one can pass in the ``radial_velocity``
+keyword argument if one uses ``differential_type="radial"``.


Test patch:
diff --git a/astropy/coordinates/tests/test_regression.py b/astropy/coordinates/tests/test_regression.py
index bf9e042ab8..5c94e3aae9 100644
--- a/astropy/coordinates/tests/test_regression.py
+++ b/astropy/coordinates/tests/test_regression.py
@@ -751,3 +751,39 @@ def test_regression_10291():
     assert_quantity_allclose(
         venus.separation(sun), 554.427 * u.arcsecond, atol=0.001 * u.arcsecond
     )
+
+
+@pytest.mark.parametrize("coord_cls", [ICRS, SkyCoord])
+@pytest.mark.parametrize(
+    "differential_type, diff_kwargs",
+    [
+        (None, {}),
+        (
+            "unitsphericalcoslat",
+            {
+                "pm_ra_cosdec": [40, 50] * u.mas / u.yr,
+                "pm_dec": [60, 70] * u.mas / u.yr,
+            },
+        ),
+        ("radial", {"radial_velocity": [80, 90] * u.km / u.s}),
+    ],
+)
+@pytest.mark.parametrize("extra_kwargs", [{}, {"representation_type": "unitspherical"}])
+def test_regression_16998(coord_cls, differential_type, diff_kwargs, extra_kwargs):
+    """Direct tests of the underlying problem causing gh-16998.
+
+    Note that the issue itself was of columns missing in data written to a file.
+    That is now tested directly by the "icrs" column in
+    astropy/io/tests/mixin_columns.py.
+    Here, we test the underlying problem, that .info._represent_as_dict()
+    did not return a full set of columns in some cases.
+    """
+    if extra_kwargs and differential_type:
+        extra_kwargs["differential_type"] = differential_type
+    coord = coord_cls([0, 10] * u.deg, [20, 30] * u.deg, **diff_kwargs, **extra_kwargs)
+    expected_entries = {"ra", "dec", "representation_type"}
+    if differential_type:
+        expected_entries |= {"differential_type"} | set(diff_kwargs)
+    if coord_cls is SkyCoord:
+        expected_entries.add("frame")
+    assert set(coord.info._represent_as_dict()) == expected_entries
diff --git a/astropy/io/tests/mixin_columns.py b/astropy/io/tests/mixin_columns.py
index 57418dd9cc..8c2ab9f1a5 100644
--- a/astropy/io/tests/mixin_columns.py
+++ b/astropy/io/tests/mixin_columns.py
@@ -46,7 +46,14 @@ scpmrv = coordinates.SkyCoord(
 scrv = coordinates.SkyCoord(
     [1, 2], [3, 4], [5, 6], unit="deg,deg,pc", radial_velocity=[11, 12] * u.km / u.s
 )
-icrs = coordinates.ICRS([1, 2] * u.deg, [3, 4] * u.deg)
+# Use UnitSpherical and Radial for a regression test for gh-16998.
+icrs = coordinates.ICRS(
+    [1, 2] * u.deg,
+    [3, 4] * u.deg,
+    radial_velocity=[5, 6] * u.km / u.s,
+    representation_type="unitspherical",
+    differential_type="radial",
+)
 altaz = coordinates.AltAz(
     [1, 2] * u.deg,
     [3, 4] * u.deg,
@@ -163,7 +170,13 @@ compare_attrs = {
         "representation_type",
         "frame.name",
     ],
-    "icrs": ["ra", "dec", "representation_type"],
+    "icrs": [
+        "ra",
+        "dec",
+        "radial_velocity",
+        "representation_type",
+        "differential_type",
+    ],
     "altaz": ["alt", "az", "obstime", "location", "representation_type"],
     "so": ["lon", "lat", "rotation", "origin"],
     "sond": ["rotation", "origin"],
@@ -216,7 +229,7 @@ non_trivial_names = {
         "scpmrv.radial_velocity",
     ],
     "scrv": ["scrv.ra", "scrv.dec", "scrv.distance", "scrv.radial_velocity"],
-    "icrs": ["icrs.ra", "icrs.dec"],
+    "icrs": ["icrs.ra", "icrs.dec", "icrs.radial_velocity"],
     "altaz": [
         "altaz.az",
         "altaz.alt",
@@ -234,7 +247,12 @@ non_trivial_names = {
         "so.origin.location.y",
         "so.origin.location.z",
     ],
-    "sond": ["sond.rotation", "sond.origin.ra", "sond.origin.dec"],
+    "sond": [
+        "sond.rotation",
+        "sond.origin.ra",
+        "sond.origin.dec",
+        "sond.origin.radial_velocity",
+    ],
     "sd": ["sd.d_lon_coslat", "sd.d_lat", "sd.d_distance"],
     "sr": ["sr.lon", "sr.lat", "sr.distance"],
     "srd": [