/matplotlib

Give a better error message on missing PostScript fonts #6428

Merged
merged 2 commits into from Dec 19, 2016
+80 −6

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
not a blocker
Projects
None yet
Milestone
  2.1 (next point r...
6 participants
@jkseppan @jenshnielsen @NelleV @tacaswell @mdboom @QuLogic
@jkseppan
Member

jkseppan commented May 15, 2016

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.

mdboom added the needs_review label May 15, 2016

tacaswell added this to the 1.5.2 (Critical bug fix release) milestone May 16, 2016

tacaswell added the not a blocker label May 16, 2016

@tacaswell tacaswell modified the milestone: 2.0 (style change major release), 1.5.2 (Critical bug fix release) May 16, 2016

jenshnielsen closed this May 22, 2016

jenshnielsen reopened this May 22, 2016

@jenshnielsen jenshnielsen added needs_review and removed needs_review labels May 22, 2016

@jenshnielsen
Owner

jenshnielsen commented May 22, 2016

Cycled to retrigger appveyor I think the error is unrelated.
@jenshnielsen
Owner

jenshnielsen commented May 22, 2016

👍 looks good to me
@jkseppan
Member

jkseppan commented May 22, 2016

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
Member

jkseppan commented May 22, 2016

trying to retrigger appveyor

jkseppan closed this May 22, 2016

jkseppan reopened this May 22, 2016

@mdboom mdboom added needs_review and removed needs_review labels May 22, 2016

@tacaswell tacaswell modified the milestone: 2.1 (next point release), 2.0 (style change major release) May 22, 2016

@tacaswell tacaswell commented on the diff May 22, 2016

View full changes
lib/matplotlib/backends/backend_pdf.py
@@ -2523,6 +2535,7 @@ def print_pdf(self, filename, **kwargs):
bbox_inches_restore=_bbox_inches_restore)
self.figure.draw(renderer)
renderer.finalize()
+ file.finalize()
@tacaswell

tacaswell May 22, 2016

Owner

Why did this get added here?

@jkseppan

jkseppan May 23, 2016

Member

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 some commits May 15, 2016

@jkseppan jkseppan Separate pdf finalization from closing 
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
 433b899
@jkseppan jkseppan Give a better error message on missing PostScript fonts 
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.

All checks have passed

3 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.2%) to 69.953%
Details
4fcc0e7
@jkseppan
Member

jkseppan commented May 23, 2016

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

NelleV commented Dec 19, 2016 • edited

LGTM 👍

NelleV changed the title from Give a better error message on missing PostScript fonts to [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
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 69.953%
Details

QuLogic changed the title from [MRG+1] Give a better error message on missing PostScript fonts to Give a better error message on missing PostScript fonts Dec 19, 2016

QuLogic removed the needs_review label Dec 19, 2016

QuLogic referenced this pull request Dec 22, 2016

Merged

Save SVG test directly to file instead of its name. #7668

jkseppan deleted the jkseppan:missing-ps-fonts branch Dec 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment