Tweak _ConverterError reporting. #22791
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
```
oscargus
approved these changes
Apr 6, 2022
|
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 8000 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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).