Add units to a Table

[weaverba137]

This PR addresses parts of #191, focussed on adding units to a astropy.table.Table.

Here's a simple test snippet:

import os
import importlib.resources as ir
from astropy.table import Table
import desiutil.annotate as da
from desiutil.log import get_logger, DEBUG
da.log = get_logger(DEBUG)
csvFile = os.path.join(str(ir.files('desidatamodel')), 'data', 'column_descriptions.csv')
units, comments = da.csv_units(csvFile)
coadd = '/global/cfs/cdirs/desi/public/edr/spectro/redux/fuji/healpix/sv3/dark/99/9994/coadd-sv3-dark-9994.fits'
t = Table.read(coadd, hdu='FIBERMAP')
tt = da.annotate_table(t, units)  # tt is a copy of t; t is not supposed to be modified in this example.
# Try writing out tt.

TO DO:

• Add additional unit tests.
• Operational testing; extending the snippet above.

[coveralls]

coverage: 74.314% (-0.08%) from 74.392% when pulling 466159f on fits-units into 5d2b9d2 on main.

[sbailey]

Thanks for adding this. I added a number of comments inline related to the user interface and maintainability. Most of these are minor/optional, but curiously the one I care most about is not making the user set the logger after import since it moves boilerplate responsibility to the user instead of the model; it is a different pattern than most of the rest of our code; and science users using this for a VAC may not even know what a desiutil logger is...

[sbailey]

consider adding an equivalent units check to csv_units and yml_units to catch invalid units at the time they are loaded, before attempting to apply them to a table.

OTOH, it if the input file has invalid units for a column that isn't in the table anyway, it could be considered a feature to not complain until you actually try to use it, so I could see an argument either way.

[weaverba137]

Right, but that would come into play when implementing the command-line script that adds units to FITS files, as opposed to a Table.

However, I should emphasize here that the UnitConversionError exception isn't checking for the validity of the unit format. That is catching the much more specific case where existing units are being converted to other units requested by the user, for example, nm --> Angstrom or deg --> arcsec.

Still, I agree that there should be a check on the validity of the unit format.

[weaverba137]

@sbailey, I suggest moving forward with merging this and splitting off the task of verifying unit format into a separate issue.

There is already unit format verification code in desidatamodel, and I additionally propose moving that code to desiutil so that desiutil doesn't have to depend on desidatamodel.