Use Python built-in exception types

[colinpalmer]

It'd be very helpful if GEMMI could raise standard Python exception types when they're appropriate. The particular example we've found is trying to open a file that doesn't exist:

>>> import gemmi
>>> gemmi.cif.read('non-existent_file.star')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: unable to open() file non-existent_file.star for reading errno 2

It would help us to write much more natural Python code if we could catch this as a standard FileNotFoundError rather than having to catch RuntimeError and look into the message to identify the problem.

[wojdyr]

Yes, it's a good point.
Did you spot any other exceptions that could be used apart from FileNotFoundError?

[colinpalmer]

Yes, there are a few others. It's a bit awkward to keep Python 2 support because the exceptions there are different. Here are a few that I've noticed to start with:

Error	Python 2 exception	Python 3 exception
File not found	IOError	FileNotFoundError
Permission problems	IOError	PermissionError
Try to open directory as file	IOError	IsADirectoryError

Also, if GEMMI can open a file but there's a problem parsing it, maybe it should raise ValueError instead of RuntimeError? But that's a bit more debatable.

@d-j-hatton has been using this too. Dan, have you noticed any other errors where a more specific type would be useful?

[wojdyr]

That's more complex than I thought. For this, I suppose I'd need to check errno after calling fopen() or ifstream/ofstream.

Maybe I could just raise IOError? Would it help? In C++ I'd probably use std::system_error.
If I'd manage to do it in pybind11, IOError would contain errno that denotes the actual problem.

[colinpalmer]

Yes, just using IOError in all of those cases would definitely help, and is valid in Python 2 and 3.

[Anthchirp]

Valid in Python 2, but it may be less useful in Python 3 as it would probably not be caught by an except FileNotFoundError block?

[wojdyr]

but why would you prefer writing

except FileNotFoundError
except PermissionError
except IsADirectoryError
etc.

instead of a single except IOError?

[colinpalmer]

If you intend to handle one or more of those cases in your own code, rather than just passing the message on to the user.

In our case, FileNotFoundError often just means the file hasn't appeared through NFS yet (or is still being copied by rsync, or being rewritten by another process, etc). So, if we know it's a FileNotFoundError we can just wait and try again, but if we get other types of IOError we probably have to stop and report a failure.

As I said, I think IOError would be a useful improvement over RuntimeError, but using the "correct" Python 3 exception types would be even better.

[d-j-hatton]

The only case where we need to catch the exception so far is when the file we want isn't there for one of the reasons mentioned by Colin. It may also be possible that we sometimes get partially written files which give you something like

RuntimeError: /path/to/file: Wrong number of values in the loop

which we also want to catch.

At the moment we would be catching that with the except RuntimeError that we use to catch the case where the file isn't found so I don't know if that happens often (or at all). To Colin's earlier point that would arguably be a ValueError.

So I suppose the ideal case is to have FileNotFoundError for the former and ValueError for the latter but having IOError rather than RuntimeError would be a helpful improvement for the former.

[wojdyr]

OK, I started working on it and will probably finish next week.

BTW, if you are automating thing in DLS have a look at #116
(do people who maintain MX pipelines at synchrotrons have any forum or mailing list?)

[wojdyr]

I changed file reading errors and a couple of other ones to IOError.
It turns out that IOError(2, 'message') actually returns FileNotFoundError, and similarly PermissionError etc are handled automatically. So this is done. Apart from the filename property which is missing. Not ideal, but passing filename around would require a custom C++ exception, which I wanted to avoid.

Regarding cif parsing errors, I think ideally we would have exception similar to json.JSONDecodeError, but creating custom exceptions with extra properties is not straightforward in pybind11, so in the end I used ValueError as you proposed.

[colinpalmer]

Great, that all sounds good. Thanks!