Repository navigation
Follow-up for GSOC 2021: MRT writer in io.ascii #12130
Description
In #11897, @Suyog7130 implemented a MRT writer for io.ascii. That is a very long PR, with >> 100 comments already and it has other PRs depending on it. To get this moving and to avoid excessive rebasing, we decided to split out a few comments about minor functionality, stylistic reworks and code improvements into this new issue.
Below is a list of thinks that we can tick off as they are addresses in follow-up PRs:
- The rules that decide the write-out precision might drop precision in some cases (Add MRT format writer to
cds.py[GSoC21a] #11897 (comment)) - A large block of code could be factored out into a new function, also renaming a variable to be more explicit (Add MRT format writer to
cds.py[GSoC21a] #11897 (comment)) - The current implementation does not work with
QTable(Add MRT format writer tocds.py[GSoC21a] #11897 (comment)) - The default description for columns with no description set can be shortened (Add MRT format writer to
cds.py[GSoC21a] #11897 (comment)) - The conversion float/string can be fooled if the float is formatted into something that does not look like a float any longer (Add MRT format writer to
cds.py[GSoC21a] #11897 (comment)) - split format into MRT and CDS writers.
- there was some workaround (but I can't find the line now) for a numpy deprecation warning. That workaround can be removed when support for numpy 1.18 is dropped.
Activity
taldcroft commentedon Aug 30, 2021
Is there a separate issue for splitting the format into
CDSandMRTclasses with distinct format names, or will that be tracked here?hamogu commentedon Aug 30, 2021
I'll add it to the list.
Suyog7130 commentedon Aug 31, 2021
@hamogu Thank you very much for opening up this issue.
Suyog7130 commentedon Oct 30, 2021
Regarding #12302 (comment)
Earlier we hadn't set any default number of digits and different machines, linux/Windows/Mac, computed values with different precision, resulting in test failures. We decided upon the pico-arcsecond precision as the hard-coded default, arguing that the user would prefer a higher default precision. They have the option to strip the number of digits using
formatif required. But default 3-6 digit precision is indeed good from the perspective of MRT tables which are used in publications. Whereas, since CDS tables are typically saved in the Vizier database, so there the usage requirement may differ.I have a small suggestion.
As you say, we can have a keyword in
cdsdict/mrtdict(or a direct keyword to thewritefunction) for the purpose of setting precision of the coordinate columns after splitting. We also ultimately want the division into component columns to work for tables with multiple coordinate columns, with the resultant components being numbered likeRAs_1,RAs_2... etc. The keyword argument can be used to set arcsecond precision of all the coordinate columns.