Tweak _ConverterError reporting.#22791
Conversation
Chaining an empty _ConverterError doesn't add useful info; instead,
print out the output stream.
e.g. modify _GSConverter to use the invalid `-sDEVICE=1234`; previously
`_GSConverter()("foo", "bar")` would give:
```
Traceback (most recent call last):
File ".../matplotlib/testing/compare.py", line 110, in __call__
self._read_until(b"\nGS")
File ".../matplotlib/testing/compare.py", line 95, in _read_until
raise _ConverterError
matplotlib.testing.compare._ConverterError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/antony/src/extern/matplotlib/lib/matplotlib/testing/compare.py", line 112, in __call__
raise OSError("Failed to start Ghostscript") from err
OSError: Failed to start Ghostscript
```
or, if just passing buf as parameter when constructing the
_ConverterError:
```
Traceback (most recent call last):
File ".../matplotlib/testing/compare.py", line 110, in __call__
self._read_until(b"\nGS")
File ".../matplotlib/testing/compare.py", line 95, in _read_until
raise _ConverterError(buf)
matplotlib.testing.compare._ConverterError: bytearray(b'GPL Ghostscript 9.55.0 (2021-09-27)\nCopyright (C) 2021 Artifex Software, Inc. All rights reserved.\nThis software is supplied under the GNU AGPLv3 and comes with NO WARRANTY:\nsee the file COPYING for details.\nUnknown device: 1234\n')
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File ".../matplotlib/testing/compare.py", line 112, in __call__
raise OSError("Failed to start Ghostscript") from err
OSError: Failed to start Ghostscript
```
and now it gives:
```
Traceback (most recent call last):
File "<string>", line 1, in <module>
File ".../matplotlib/testing/compare.py", line 112, in __call__
raise OSError("Failed to start Ghostscript:\n\n" + err.args[0]) from None
OSError: Failed to start Ghostscript:
GPL Ghostscript 9.55.0 (2021-09-27)
Copyright (C) 2021 Artifex Software, Inc. All rights reserved.
This software is supplied under the GNU AGPLv3 and comes with NO WARRANTY:
see the file COPYING for details.
Unknown device: 1234
```
There was a problem hiding this comment.
Would be nice with tests, but not a dealbreaker.
|
I did think very briefly about that, but didn't have an easy way to cause e.g. inkscape or ghostscript init to fail (which would be needed to test that this works fine). Complex contraptions are possible (e.g. make the command line args passed to either process be configurable and intentionally pass in incorrect args), but this seems a bit too complicated to be worth it? |
|
I agree that while testing everything is good, in this case this is sad-paths in calling external programs as part of our testing. Given that this is not general user facing, if you are running this your are already doing something related to development work, and even with the better error messages you are in for a debugging session one way or the other I'm comfortable merging this as-is. |
|
OK! I was under the impression that one could simply do something like the example. |
Chaining an empty _ConverterError doesn't add useful info; instead,
print out the output stream.
e.g. modify _GSConverter to use the invalid
-sDEVICE=1234; previously_GSConverter()("foo", "bar")would give:or, if just passing buf as parameter when constructing the
_ConverterError:
and now it gives:
Inspired by #22768; should probably wait for that one to go in first to prevent a rebase, unless @tacaswell is OK with doing the rebase in the other direction.
PR Summary
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).