Skip to content

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

Merged
merged 2 commits into from
Dec 19, 2016

Conversation

jkseppan
Copy link
Member

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.

@tacaswell tacaswell added this to the 1.5.2 (Critical bug fix release) milestone May 16, 2016
@tacaswell tacaswell modified the milestones: 2.0 (style change major release), 1.5.2 (Critical bug fix release) May 16, 2016
@jenshnielsen jenshnielsen reopened this May 22, 2016
@jenshnielsen
Copy link
Member

Cycled to retrigger appveyor I think the error is unrelated.

@jenshnielsen
Copy link
Member

👍 looks good to me

@jkseppan
Copy link
Member Author

The appveyor error does seem unrelated:

Downloading from http://repo.continuum.io/miniconda/Miniconda-latest-Windows-x86.exe
...
urllib.ContentTooShortError: retrieval incomplete: got only 57512 out of 30436432 bytes

@jkseppan
Copy link
Member Author

trying to retrigger appveyor

@jkseppan jkseppan closed this May 22, 2016
@jkseppan jkseppan reopened this May 22, 2016
@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.0 (style change major release) May 22, 2016
@@ -2523,6 +2535,7 @@ def print_pdf(self, filename, **kwargs):
bbox_inches_restore=_bbox_inches_restore)
self.figure.draw(renderer)
renderer.finalize()
file.finalize()
Copy link
Member

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?

Copy link
Member Author

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.

jkseppan added 2 commits May 23, 2016 12:30
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.
@jkseppan jkseppan force-pushed the missing-ps-fonts branch from c820a5d to 4fcc0e7 Compare May 23, 2016 09:31
@jkseppan
Copy link
Member Author

Rebased in the hope that the AppVeyor change in master makes the tests more robust.

@NelleV
Copy link
Member

NelleV commented Dec 19, 2016

LGTM 👍

@NelleV NelleV changed the title Give a better error message on missing PostScript fonts [MRG+1] Give a better error message on missing PostScript fonts Dec 19, 2016
@tacaswell tacaswell merged commit f99a985 into matplotlib:master Dec 19, 2016
@QuLogic QuLogic changed the title [MRG+1] Give a better error message on missing PostScript fonts Give a better error message on missing PostScript fonts Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants