Skip to content

Delay checking for existence of postscript distillers. #16537

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 1 commit into from
Mar 10, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Feb 16, 2020

Currently, if one has ps.usedistiller set in their matplotlibrc,
matplotlib will immediately check for the presence of ghostscript (and
possibly pstopdf) at import time and run them in subprocesses to check
their version, even though they may not be needed at all (if the user is
not going to save a postscript image); this can quite slow down import
time. Compare e.g. with text.usetex which requires the presence of
a tex install, but doesn't check for its presence until it is actually
needed.

Delay the checking for the presence of the distillers until we actually
need them. For consistency with the old behavior, don't error out if
they are missing, but just emit a warning.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@@ -1239,6 +1250,9 @@ def xpdf_distill(tmpfile, eps=False, ptype='letter', bbox=None, rotated=False):
operators. This distiller is preferred, generating high-level postscript
output that treats text as text.
"""
mpl._get_executable_info("gs")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check be up around line 1218?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is for ps2pdf (which is actually just a thin wrapper script around gs). Added a comment clarifying.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't gs_distill need an equivalent check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already calls mpl._get_executable_info("gs") basically at the beginning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was looking for it on it's own line an not parsing that it was called in-line to another function call 🐑

@anntzer anntzer force-pushed the delaydistillercheck branch from cab5463 to 158f6e0 Compare February 16, 2020 20:08
Currently, if one has `ps.usedistiller` set in their matplotlibrc,
matplotlib will immediately check for the presence of ghostscript (and
possibly pstopdf) at import time and run them in subprocesses to check
their version, even though they may not be needed at all (if the user is
not going to save a postscript image); this can quite slow down import
time.  Compare e.g. with `text.usetex` which requires the presence of
a `tex` install, but doesn't check for its presence until it is actually
needed.

Delay the checking for the presence of the distillers until we actually
need them.  For consistency with the old behavior, don't error out if
they are missing, but just emit a warning.
@anntzer anntzer force-pushed the delaydistillercheck branch from 158f6e0 to 926c347 Compare February 17, 2020 06:09
@QuLogic QuLogic merged commit a1cd024 into matplotlib:master Mar 10, 2020
@anntzer anntzer deleted the delaydistillercheck branch March 10, 2020 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants