More helpful error message for pgf backend #4022
Conversation
Makes the error message more helpful if subprocess fails with a file-not-found-error.
| raise RuntimeError("Error starting process '%s'" % self.texcommand) | ||
| except OSError as e: | ||
| if e.errno == errno.ENOENT: | ||
| raise OSError(errno.ENOENT, "Latex command not found. " |
There was a problem hiding this comment.
Is there a good reason to not re-raise this as an RuntimeError? By changing the exception type we are breaking API (as there is probably user code which expects to catch RuntimeError here).
There was a problem hiding this comment.
I guess the idea was to discriminate between the "command not found" and "command returned with error" case, so errors when running the latex process are propagated as RuntimeError, not finding the latex command is an OS or environment error. That distinction is a new "feature" if you would like to say so. I don't really know if this is to be considered as breaking the API, since the behaviour on error is neither defined nor documented anywhere.
|
I'm confused.. didn't I apply such a patch to master already? Can't find it though.. |
|
No wait, I was the one catching and re-raising the OSError as RuntimeError with an error string (#3158). Do you feel that these additional changes to the error string are necessary? |
|
TLDR: I do not care which error type you want to have but I think it would be nice if we could provide a help if the cause of the error is almost certain and I'm happy to change my PR to RuntimeError. Until “recently” (well F21 still ships an older version) it was an uncomprehensible uncatched error message and without googling stackoverflow I would not have any idea what as going on… http://stackoverflow.com/questions/20000427/matplotlib-pgf-oserror-no-such-file-or-directory-in-subprocess-py Because of that I also heavily doubt that anybody relies on the fact that the error is an RuntimeError. Thus I thought it would be nice to discriminate between some random error an a specific error whose cause is known. Actually I think that this is a issue in python itself. It is not helpful at all to raise an file-not-found error without mentioning the requested file command. |
|
Fair enough, target 1.5.x (no backporting). I assumed that the Can you add a note in https://github.com/matplotlib/matplotlib/tree/master/doc/api/api_changes I agree it seems pedantic, but I think it is worth doing. |
|
I thought about it, converting to RuntimeError is probably a good idea because people will maybe catch an OSError to check if their (data) files are missing. Sorry for the discussion I'll change it. ;) |
|
Thanks for this enhancement! |
More helpful error message for pgf backend
Makes the error message more helpful if subprocess fails with a file-not-found-error.