Repository navigation
astropy.io.ascii.Cds format exception (ValueError for invalid NULL) #11239
Description
EDIT: Solution provided at #11239 (comment)
Description
The CDS format (astropy.io.ascii.Cds) throws exceptions for some VizieR tables.
The exception appears for ascii tables having '-' as null values.
Expected behavior
Take into account the '-' as null values in the read method.
Actual behavior
Throw an Exception
Steps to Reproduce
# Put your Python code snippet here.
from astropy.io import ascii
readme = "ftp://cdsarc.u-strasbg.fr/pub/cats/J/A+A/642/A176/ReadMe"
r = ascii.get_reader(ascii.Cds, readme=readme)
table = r.read("ftp://cdsarc.u-strasbg.fr/pub/cats/J/A+A/642/A176/table5.dat")
Activity
pllim commentedon Jan 12, 2021
The exception is
InconsistentTableError: Can't find table contentsfrom astropy 4.3.dev . (I did usedownload_fileto cache both files first, so maybe I am hitting a different error...)From the readme:
Table data
taldcroft commentedon Jan 12, 2021
The problem is that the
table5.datfile is not compliant with the CDS data standard. It uses'-'to indicate NULL values, whereas the standard requires a blank field. This should be reported to the data provider. That table also has bad values for the units, but these don't cause anything more than warnings.Fortunately there is a workaround:
pllim commentedon Jan 12, 2021
I was just gonna post an updated traceback for the record. I did get a different traceback if I
curlthe files first to working directory. I am glad @taldcroft found a solution already![-]astropy.io.ascii.Cds format exception[/-][+]astropy.io.ascii.Cds format exception (ValueError for invalid NULL)[/+]taldcroft commentedon Jan 12, 2021
@gilleslandais - are you OK closing this issue? I think astropy is behaving correctly by reporting that it expects integer values in that column based on the header and got a
-instead.mhvk commentedon Jan 14, 2021
@gilleslandais - the solution by @taldcroft is a good one, and it sounds like it would be nice if it were documented somewhere... Obviously, the trouble is how to both make it discoverable yet not distract too much from the flow of the more common usage. Is there a specific place in the documentation where you looked that would be a good place to put something like this?
gilleslandais commentedon Jan 14, 2021
There is an incoherence between our documentation and the current usage – we have to discuss about it in CDS .. and then I will get back to you..
about the unit : [] in the ReadMe byte-by-byte means log. In the table considered, the unit is [-] which means a log value on undefined unit – the rule is briefly presented in the CDS documentation.
taldcroft commentedon Jan 14, 2021
I think that https://docs.astropy.org/en/latest/api/astropy.io.ascii.Cds.html#cds would be the most discoverable place.
taldcroft commentedon Jan 14, 2021
About logarithmic units, the code should handle this a little better. AFAIK astropy units cannot deal with arbitrary log units, so the general
[unit]syntax isn't going to work. But we could detect that and issue a more useful warning. We could also special-case[-]as a do-nothing unit.gilleslandais commentedon Jan 14, 2021
yes - as information only !
mhvk commentedon Jan 14, 2021
We can deal with log10 units -
u.dex(physical_unit)- in fact, the following works:So, it may be a question of just special-casing
[-]. Should be quite easy!mhvk commentedon Jan 14, 2021
@gilleslandais - just to be sure, a single
-is not expected to occur, right? I.e., dimensionless itself would always be---.4 remaining items
pllim commentedon Jan 16, 2021
Can we close this now that #11250 is merged? Thanks, everyone!
mhvk commentedon Jan 16, 2021
The main issue was about different ways in which absent data is represented. @taldcroft provided a nice work-around, but should there perhaps be a bit more flexibility in the reader about what constitutes absent data? Or at least additional documentation?
taldcroft commentedon Jan 16, 2021
It is entirely understandable that people might not find the existing docs, or appreciate that the generalized infrastructure applies to the CDS reader, but here you go:
https://docs.astropy.org/en/latest/io/ascii/read.html#bad-or-missing-values
The existing code is actually quite flexible in what constitutes absent data, to the point of leading to an API that is probably more complicated than I would write these days.
If using
-is a commonly used but undocumented quasi-standard for missing data in CDS, then it could be worth putting the workaround I suggested into the CDS reader doc string (https://docs.astropy.org/en/latest/api/astropy.io.ascii.Cds.html#astropy.io.ascii.Cds).BTW if the byte-by-byte indicates the column with
?=-, then it should mark those as missing (otherwise would be a bug).mhvk commentedon Jan 16, 2021
Indeed. I just tried in ipython,
Table.read.help('cds')and indeed do get information that should help.Possibly, one could add another "caveat" at https://docs.astropy.org/en/latest/api/astropy.io.ascii.Cds.html#astropy.io.ascii.Cds to send people in the right direction? Something along the lines of "While most tables will read fine, sometimes it may be necessary to use some of the general reader settings, such as to setcustom fill_values".
But I'm just guessing now... @gilleslandais - was there a particular place where you tried to look for information? We always have the difficulty of trying to provide helpful information yet not making the documentation so long that the information can no longer be found! Obviously, what is to be done also depends on whether this table was effectively "buggy" by CDS standards.
p.s. I realize I bigger thing that we should (eventually) implement is writing in CDS format - that should make things quite a bit easier for people to upload data/catalogues.
gilleslandais commentedon Jan 18, 2021
The CDS documention had to be rewrite, but I confirm that the URL given in astropy https://docs.astropy.org/en/latest/api/astropy.io.ascii.Cds.html#astropy.io.ascii.Cds is the current official documentation in CDS (http://vizier.u-strasbg.fr/doc/catstd.htx)
About the option to write in CDS format - I guess you mean a serialisation from an astropy table to generate byte-by-byte+ascii aligned table ?
We provide a Python library which do it - see https://github.com/cds-astro/cds.pyreadme/ -
The library was updated recently and is now provided through github . The package is based on astropy. The process to extract the format and the process to write could certainly be improved with astropy expertise :-) in particular for large tables (I am really interested if you have suggestion)
The cdspyreadme package is dedicated for authors to submit data in VizieR (linked also in AAS documentation) – it is one of the possibility for authors (currently not the most popular) – there are others.
Having an astropy library to build byte-by-byte is an interesting idea !
taldcroft commentedon Jan 18, 2021
That's great about the CDS writer! I didn't know about that. At the very least we should include information about the writer in the astropy CDS docs. I would be very happy to discuss adapting the code to work directly within astropy, either as an optional dependency or else effectively moving the package into astropy as a subpackage of
astropy.io.ascii. This could allow people to use the existingcdspyreadmeinterface directly or just usetbl.write(... format='ascii.cds'). This is up to you @gilleslandais. There are always pros and cons of moving a lightweight little package into a big one with a longer release cycle etc.mhvk commentedon Jan 18, 2021
That's very good news indeed! It should help a lot to have code to either incorporate or have sanity checks against. Since it is a bit orthogonal to the issue here of having problems reading CDS tables, I opened a separate issue: #11257.
gilleslandais commentedon Jan 19, 2021
It's sounds good -
In CDS side we will clarify NULL decimal values-
when NULL decimal values are not specified with '?=..' , only blank are accepted. Else decimal NULL values will be declared (example: ?=999 for 999 NULL values or ?=- to use the pattern --* as NULL)
taldcroft commentedon Jan 21, 2021
Just to note that the astropy reader won't work out of the box with a regex for specifying missing values. From a standards perspective I'm happier with things being explicit and simple:
?=-means exactly-is NULL and?=---means---is NULL. But if there is a legacy of files that assume that?=-should mean--*then the CDS reader needs to find a way.taldcroft commentedon Jan 21, 2021
I had forgotten that the code does handle basically handle this case of
?=-by allowing for any of-,--,---, or----as null values. See #1335.pllim commentedon Aug 30, 2021
Hi humans 👋 - this issue was labeled as Close? approximately 7 months ago. If you think this issue should not be closed, a maintainer should remove the Close? label - otherwise, I will close this issue in 7 days.
If you believe I commented on this issue incorrectly, please report this here
pllim commentedon Aug 30, 2021
Ops... Bot comment is accurate but I used the wrong API key. 😅
github-actions commentedon Sep 7, 2021
I'm going to close this issue as per my previous message, but if you feel that this issue should stay open, then feel free to re-open and remove the Close? label.
If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here