Repository navigation
Consider catching exceptions in io.ascii readers and raising as InconsistentTableError #3346
Description
Currently in the guessing process there are four exceptions that are handled in a failed read attempt: core.InconsistentTableError, ValueError, TypeError, AttributeError. The last one was added in #3342 to fix a problem with tables that almost look like DAOphot.
A different way to approach this is to pro-actively catch expected parsing errors in the various readers and re-raise as InconsistentTableError or possibly subclasses there-of. This would have the benefit of forcing scrutiny of the various failure paths and then providing users with better feedback for what went wrong. This would be immediately beneficial in the no-guess case. It could also be useful for guessing since we might provide a mode to print out the exception message for each guess try. Currently this isn't that useful since the exceptions are not always something that the user can make sense of.
Activity
hamogu commentedon Jan 23, 2015
In the specific case of #3342 I've actually thought about catching the generic exception and raising an
InconsistentTableErrorinstead. While we can think about it for any specific case, I think it is better go with a broader approach:We don't know know what readers we might have in the future and even in the existing readers there might be many edge cases like e.g. #3342 that are not yet discovered (e.g. we obviously did not think that the reader could possibly fail this way when we wrote it otherwise that error would have been caught).
Thus, I considered - instead of listing selected exceptions - to just catch all exceptions in the guessing. After all, we are guessing and if the guess process does not find a way to read the table it will say "they using guessing=False and see what goes wrong".
In the vast majority of cases the Exception that is raised will be totally irrelevant for the user, because it is probably raised by a reader for the wrong format anyway. As a user, I want to read my table, I don't care why exactly the DAOPhot reader failed to read it (these days, many astronomers don't even know what DAOPhot is).
However, I realized tthat there would be strong objections to this approach, and thus opted for the middle ground. I think the most likely exceptions (those defined in io.ascii plus Value, Type and AttributeError) are now covered; the exceptions I can think of that we do want the user to see (IOError, Memory full, general your PC just died kind of exceptions) are not on the list and thuswill be raised.
So, I actually want to advertise catching all possible exceptions. That's future proof, low maintenance and easy to understand. I want to avoid a situation where we fix these corner cases one by one as they turn up in real live and a user opens an issue for it. First, that's annoying and second, the most likely cause of action is that the user just says "this does not work, I'll better just parse
f.readlines()myself or use my legacy IDL code").hamogu commentedon Jan 23, 2015
Looking at the top three pages of open astropy issues (75 issues, of which maybe 50 are bugs, simple feature requests etc that any user cold have and 25 are "make release", "optimize Travis" etc. ), there are <5 issues opened by people who are not developing astropy or an affiliated package. We know that the astropy user base is >> 10, so do the non-developers never run in to issues?
I believe that's not the case, but non-developers don't report issues.