-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Give a better error message on missing PostScript fonts #6428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Cycled to retrigger appveyor I think the error is unrelated. |
👍 looks good to me |
The appveyor error does seem unrelated:
|
trying to retrigger appveyor |
@@ -2523,6 +2535,7 @@ def print_pdf(self, filename, **kwargs): | |||
bbox_inches_restore=_bbox_inches_restore) | |||
self.figure.draw(renderer) | |||
renderer.finalize() | |||
file.finalize() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this get added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file.close()
call in the finally
section no longer does the finalization. The finalization is the part that cannot be reasonably done if something has raised an exception (in this case, the missing font) so it belongs in the try
section.
If an error is raised and the object state is inconsistent, we shouldn't try to finalize the file, just close all resources. The idea is to support the following idiom: try: figure.draw(file) # write pdf file, can raise file.finalize() # do this if everything went well finally: file.close() # do this in any case
For matplotlib#4167; does not fix the problem, as it would need supporting a whole different kind of font, but gives a more useful error message.
c820a5d
to
4fcc0e7
Compare
Rebased in the hope that the AppVeyor change in master makes the tests more robust. |
LGTM 👍 |
For #4167; does not fix the problem, as it would need supporting a whole different kind of font, but gives a more useful error message.