Skip to content
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

Work around missing subprocess members on Google App Engine #1825

Merged
merged 8 commits into from Mar 20, 2013
7 changes: 7 additions & 0 deletions doc/api/api_changes.rst
Expand Up @@ -32,6 +32,13 @@ Changes in 1.3.x
by ``self.vline`` for vertical cursors lines and ``self.hline`` is added
for the horizontal cursors lines.

* On POSIX platforms, the :func:`~matplotlib.cbook.report_memory` function
raises :class:`NotImplementedError` instead of :class:`OSError` if the
:command:`ps` command cannot be run.

* The :func:`~matplotlib.cbook.check_output` function has been moved to
`~matplotlib.compat.subprocess`.

Changes in 1.2.x
================

Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/__init__.py
Expand Up @@ -124,14 +124,15 @@
'.'.join(str(x) for x in _required),
sys.version_info[0]))

import os, re, shutil, subprocess, warnings
import os, re, shutil, warnings
import distutils.sysconfig
import distutils.version

# cbook must import matplotlib only within function
# definitions, so it is safe to import from it here.
from matplotlib.cbook import MatplotlibDeprecationWarning
from matplotlib.cbook import is_string_like
from matplotlib.compat import subprocess

try:
reload
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/animation.py
Expand Up @@ -20,8 +20,8 @@
import sys
import itertools
import contextlib
import subprocess
from matplotlib.cbook import iterable, is_string_like
from matplotlib.compat import subprocess
from matplotlib import verbose
from matplotlib import rcParams

Expand Down
4 changes: 2 additions & 2 deletions lib/matplotlib/backends/backend_pgf.py
Expand Up @@ -7,7 +7,6 @@
import shutil
import tempfile
import codecs
import subprocess
import atexit
import weakref

Expand All @@ -21,7 +20,8 @@
from matplotlib import font_manager
from matplotlib.ft2font import FT2Font
from matplotlib.cbook import is_string_like, is_writable_file_like
from matplotlib.cbook import check_output
from matplotlib.compat import subprocess
from matplotlib.compat.subprocess import check_output


###############################################################################
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_ps.py
Expand Up @@ -88,7 +88,7 @@ def gs_version(self):
except KeyError:
pass

from subprocess import Popen, PIPE
from matplotlib.compat.subprocess import Popen, PIPE
pipe = Popen(self.gs_exe + " --version",
shell=True, stdout=PIPE).stdout
if sys.version_info[0] >= 3:
Expand Down
72 changes: 22 additions & 50 deletions lib/matplotlib/cbook.py
Expand Up @@ -17,7 +17,6 @@
import locale
import os
import re
import subprocess
import sys
import threading
import time
Expand Down Expand Up @@ -1212,19 +1211,34 @@ def restrict_dict(d, keys):

def report_memory(i=0): # argument may go away
'return the memory consumed by process'
from subprocess import Popen, PIPE
from matplotlib.compat.subprocess import Popen, PIPE
pid = os.getpid()
if sys.platform == 'sunos5':
a2 = Popen('ps -p %d -o osz' % pid, shell=True,
stdout=PIPE).stdout.readlines()
try:
a2 = Popen('ps -p %d -o osz' % pid, shell=True,
stdout=PIPE).stdout.readlines()
except OSError:
raise NotImplementedError(
Copy link
Member

Choose a reason for hiding this comment

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

I'm tempted to suggest a RuntimeError here. None-the-less, this is an API change which should be documented in docs/api/api_changes.rst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Note that I am having trouble building the docs locally, so I'm hoping my ReST syntax is correct!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I agree RuntimeError is more appropriate (since NotImplementedError implies that it is planning to be fixed one day), but I changed this to be compatible with the Windows implementation. Happy to change it if you want. For what it's worth, NotImplementedError is a subclass of RuntimeError.

"report_memory works on Sun OS only if "
"the 'ps' program is found")
mem = int(a2[-1].strip())
elif sys.platform.startswith('linux'):
a2 = Popen('ps -p %d -o rss,sz' % pid, shell=True,
stdout=PIPE).stdout.readlines()
try:
a2 = Popen('ps -p %d -o rss,sz' % pid, shell=True,
stdout=PIPE).stdout.readlines()
except OSError:
raise NotImplementedError(
"report_memory works on Linux only if "
"the 'ps' program is found")
mem = int(a2[1].split()[1])
elif sys.platform.startswith('darwin'):
a2 = Popen('ps -p %d -o rss,vsz' % pid, shell=True,
stdout=PIPE).stdout.readlines()
try:
a2 = Popen('ps -p %d -o rss,vsz' % pid, shell=True,
stdout=PIPE).stdout.readlines()
except OSError:
raise NotImplementedError(
"report_memory works on Mac OS only if "
"the 'ps' program is found")
mem = int(a2[1].split()[0])
elif sys.platform.startswith('win'):
try:
Expand Down Expand Up @@ -1795,45 +1809,3 @@ def get_instancemethod(self):
else:
def _putmask(a, mask, values):
return np.copyto(a, values, where=mask)


def _check_output(*popenargs, **kwargs):
r"""Run command with arguments and return its output as a byte
string.

If the exit code was non-zero it raises a CalledProcessError. The
CalledProcessError object will have the return code in the
returncode
attribute and output in the output attribute.

The arguments are the same as for the Popen constructor. Example::

>>> check_output(["ls", "-l", "/dev/null"])
'crw-rw-rw- 1 root root 1, 3 Oct 18 2007 /dev/null\n'

The stdout argument is not allowed as it is used internally.
To capture standard error in the result, use stderr=STDOUT.::

>>> check_output(["/bin/sh", "-c",
... "ls -l non_existent_file ; exit 0"],
... stderr=STDOUT)
'ls: non_existent_file: No such file or directory\n'
"""
if 'stdout' in kwargs:
raise ValueError('stdout argument not allowed, it will be overridden.')
process = subprocess.Popen(stdout=subprocess.PIPE, *popenargs, **kwargs)
output, unused_err = process.communicate()
retcode = process.poll()
if retcode:
cmd = kwargs.get("args")
if cmd is None:
cmd = popenargs[0]
raise subprocess.CalledProcessError(retcode, cmd, output=output)
return output


# python2.7's subprocess provides a check_output method
if hasattr(subprocess, 'check_output'):
check_output = subprocess.check_output
else:
check_output = _check_output
Empty file.
77 changes: 77 additions & 0 deletions lib/matplotlib/compat/subprocess.py
@@ -0,0 +1,77 @@
"""
A replacement wrapper around the subprocess module, with a number of
work-arounds:
- Provides the check_output function (which subprocess only provides from Python
2.7 onwards).
- Provides a stub implementation of subprocess members on Google App Engine
(which are missing in subprocess).

Instead of importing subprocess, other modules should use this as follows:

from matplotlib.compat import subprocess

This module is safe to import from anywhere within matplotlib.
"""

from __future__ import absolute_import # Required to import subprocess
from __future__ import print_function

import subprocess

__all__ = ['Popen', 'PIPE', 'STDOUT', 'check_output']


if hasattr(subprocess, 'Popen'):
Popen = subprocess.Popen
# Assume that it also has the other constants.
PIPE = subprocess.PIPE
STDOUT = subprocess.STDOUT
else:
# In restricted environments (such as Google App Engine), these are
# non-existent. Replace them with dummy versions that always raise OSError.
def Popen(*args, **kwargs):
raise OSError("subprocess.Popen is not supported")
PIPE = -1
STDOUT = -2


def _check_output(*popenargs, **kwargs):
r"""Run command with arguments and return its output as a byte
string.

If the exit code was non-zero it raises a CalledProcessError. The
CalledProcessError object will have the return code in the
returncode
attribute and output in the output attribute.

The arguments are the same as for the Popen constructor. Example::

>>> check_output(["ls", "-l", "/dev/null"])
'crw-rw-rw- 1 root root 1, 3 Oct 18 2007 /dev/null\n'

The stdout argument is not allowed as it is used internally.
To capture standard error in the result, use stderr=STDOUT.::

>>> check_output(["/bin/sh", "-c",
... "ls -l non_existent_file ; exit 0"],
... stderr=STDOUT)
'ls: non_existent_file: No such file or directory\n'
"""
if 'stdout' in kwargs:
raise ValueError('stdout argument not allowed, it will be overridden.')
process = Popen(stdout=PIPE, *popenargs, **kwargs)
output, unused_err = process.communicate()
retcode = process.poll()
if retcode:
cmd = kwargs.get("args")
if cmd is None:
cmd = popenargs[0]
raise subprocess.CalledProcessError(retcode, cmd, output=output)
return output


# python2.7's subprocess provides a check_output method
if hasattr(subprocess, 'check_output'):
check_output = subprocess.check_output
else:
check_output = _check_output
2 changes: 1 addition & 1 deletion lib/matplotlib/dviread.py
Expand Up @@ -23,9 +23,9 @@
import errno
import matplotlib
import matplotlib.cbook as mpl_cbook
from matplotlib.compat import subprocess
import numpy as np
import struct
import subprocess
import sys

if sys.version_info[0] >= 3:
Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/font_manager.py
Expand Up @@ -43,7 +43,7 @@
see license/LICENSE_TTFQUERY.
"""

import os, sys, subprocess, warnings
import os, sys, warnings
try:
set
except NameError:
Expand All @@ -54,6 +54,7 @@
from matplotlib import rcParams, get_configdir
from matplotlib.cbook import is_string_like
import matplotlib.cbook as cbook
from matplotlib.compat import subprocess
from matplotlib.fontconfig_pattern import \
parse_fontconfig_pattern, generate_fontconfig_pattern

Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/testing/compare.py
Expand Up @@ -7,6 +7,7 @@
from __future__ import division

import matplotlib
from matplotlib.compat import subprocess
from matplotlib.testing.noseclasses import ImageComparisonFailure
from matplotlib.testing import image_util, util
from matplotlib import _png
Expand All @@ -17,7 +18,6 @@
import os
import numpy as np
import shutil
import subprocess
import sys
from functools import reduce

Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/testing/util.py
@@ -1,5 +1,5 @@
import subprocess
import sys
from matplotlib.compat import subprocess


class MiniExpect:
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/tests/test_backend_pgf.py
Expand Up @@ -2,12 +2,12 @@

import os
import shutil
import subprocess
import numpy as np
import nose
from nose.plugins.skip import SkipTest
import matplotlib as mpl
import matplotlib.pyplot as plt
from matplotlib.compat import subprocess
from matplotlib.testing.compare import compare_images, ImageComparisonFailure
from matplotlib.testing.decorators import _image_directories

Expand Down
12 changes: 8 additions & 4 deletions lib/matplotlib/texmanager.py
Expand Up @@ -41,7 +41,6 @@
import os
import shutil
import sys
from subprocess import Popen, PIPE, STDOUT

from hashlib import md5

Expand All @@ -51,6 +50,7 @@
from matplotlib import rcParams
from matplotlib._png import read_png
from matplotlib.cbook import mkdirs
from matplotlib.compat.subprocess import Popen, PIPE, STDOUT
import matplotlib.dviread as dviread
import re

Expand All @@ -63,8 +63,12 @@


def dvipng_hack_alpha():
p = Popen('dvipng -version', shell=True, stdin=PIPE, stdout=PIPE,
stderr=STDOUT, close_fds=(sys.platform != 'win32'))
try:
p = Popen('dvipng -version', stdin=PIPE, stdout=PIPE, stderr=STDOUT,
close_fds=(sys.platform != 'win32'))
except OSError:
mpl.verbose.report('No dvipng was found', 'helpful')
return False
Copy link
Member

Choose a reason for hiding this comment

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

Is this a good idea? Previously an exception would have been raised, now False is returned. What impact does that have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to back off on this since it doesn't block texmanager from being imported, and it won't work anyway if dvipng is missing.

I don't think dvipng_hack_alpha is a command that should be used externally. It's used within the module to decide whether to change the output from dvipng. It looks like matplotlib detects the presence of dvipng on startup and if it isn't present, it disables the use of the tex module anyway. So this shouldn't really have an effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you want me to revert this, or document it, or just leave as-is?

stdin, stdout = p.stdin, p.stdout
for line in stdout:
if line.startswith(b'dvipng '):
Expand All @@ -74,7 +78,7 @@ def dvipng_hack_alpha():
version = version.decode('ascii')
version = distutils.version.LooseVersion(version)
return version < distutils.version.LooseVersion('1.6')
mpl.verbose.report('No dvipng was found', 'helpful')
mpl.verbose.report('Unexpected response from dvipng -version', 'helpful')
return False


Expand Down
1 change: 1 addition & 0 deletions setupext.py
Expand Up @@ -494,6 +494,7 @@ def get_packages(self):
'matplotlib',
'matplotlib.backends',
'matplotlib.backends.qt4_editor',
'matplotlib.compat',
'matplotlib.projections',
'matplotlib.sphinxext',
'matplotlib.testing',
Expand Down