Avoid duplicate storage of info in serialized columns.

[mhvk]

As is, info is store on any primary data column, but also in the serialized column, which makes the header unnecessarily long. (Saw this while working on #12589, and includes a bit of the other small refactoring there.)

I think this is a bug, but could also call it refactoring.

Comparing this PR with current master:

from astropy.table import QTable, Column
QTable({"a": Column([1.], description='description', unit='')}).write('temp.ecsv', overwrite=True)

--> current master
# %ECSV 1.0
# ---
# datatype:
# - {name: a, datatype: float64, description: description}
# meta: !!omap
# - __serialized_columns__:
#     a:
#       __class__: astropy.units.quantity.Quantity
#       __info__: {description: description}
#       unit: !astropy.units.Unit {unit: ''}
#       value: !astropy.table.SerializedColumn {name: a}
# schema: astropy-2.0
a
1.0


--> this PR (__info__ removed)

# %ECSV 1.0
# ---
# datatype:
# - {name: a, datatype: float64, description: description}
# meta: !!omap
# - __serialized_columns__:
#     a:
#       __class__: astropy.units.quantity.Quantity
#       unit: !astropy.units.Unit {unit: ''}
#       value: !astropy.table.SerializedColumn {name: a}
# schema: astropy-2.0
a
1.0

Fixes #

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Do the proposed changes actually accomplish desired goals?
• Do the proposed changes follow the Astropy coding guidelines?
• Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
• Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
• Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
• 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.
• 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 a milestone set? Milestone must be set but astropy-bot check might be missing; do not let the green checkmark fool you.
• 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.

[taldcroft]

Good plan but I think this can be done with a smaller change footprint. At least this passes existing tests and your new test. I think the existing logic is simpler to understand since there is no recursion for the non-mixin case.

(astropy) ➜  astropy git:(main) ✗ git diff
diff --git a/astropy/table/serialize.py b/astropy/table/serialize.py
index 62d69ccf44..d7b21a0a96 100644
--- a/astropy/table/serialize.py
+++ b/astropy/table/serialize.py
@@ -134,7 +134,7 @@ def _represent_mixin_as_column(col, name, new_cols, mixin_cols,
         # MaskedColumn).  For primary data, we attempt to store any info on
         # the format, etc., on the column, but not for ancillary data (e.g.,
         # no sense to use a float format for a mask).
-        if data_attr == col.info._represent_as_dict_primary_data:
+        if is_primary := (data_attr == col.info._represent_as_dict_primary_data):
             new_name = name
             new_info = info
         else:
@@ -146,6 +146,10 @@ def _represent_mixin_as_column(col, name, new_cols, mixin_cols,
                                        and np.any(data.mask)) else Column
             new_cols.append(col_cls(data, name=new_name, **new_info))
             obj_attrs[data_attr] = SerializedColumn({'name': new_name})
+            if is_primary:
+                # Don't store info in the __serialized_columns__ dict for this column
+                # since this is redundant with info stored on new column itself.
+                info = {}
         else:
             # recurse. This will define obj_attrs[new_name].
             _represent_mixin_as_column(data, new_name, new_cols, obj_attrs)

[taldcroft]

Just an FYI, I find this to be an easier way to get ECSV output to the console without making a temp file:

qt = QTable({"a": Column([1.], description='description', unit='')})
ascii.write(qt, format='ecsv')

[mhvk]

@taldcroft - yes, that makes sense. I probably should not have combined this with the case in #12589, where I found that recursion was needed, because with structured dtypes one can no longer assume that a Column never needs to be serialized -- it now joins MaskedColumn in sometimes needing it. But probably best to keep that separate; certainly no reason to backport that.

[mhvk]

OK, went with your suggestion (though minus the walrus, since is_primary is used quite far from its definition). Better for this to-be-backported PR indeed.

[mhvk]

Hmm, failures are real -- for hdf5. Will check.

[mhvk]

OK, found that in reconstructing Masked instances, I had made a mistake, which was easily solved. Now the remaining failures are false positives: a time-out and a claim of reduced coverage. So, ready for final review!

[taldcroft]

I see your point that this will need updating when structured data are serialized, but agree that this way is better for the bug-fix backport.

[mhvk]

OK, thanks. Will merge and rebase the other PRs.

[pllim]

@meeseeksdev backport to v5.0.x