8000 Add PEP 519 support by aragilar · Pull Request #6788 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Add PEP 519 support #6788

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Added closed_tempfile for testing filename handling
  • Loading branch information
aragilar committed Sep 12, 2016
commit 76bb608c97776473c3b1c0f987c4e172cd58b386
20 changes: 20 additions & 0 deletions lib/matplotlib/testing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import inspect
import warnings
from contextlib import contextmanager
import shutil
import tempfile

import matplotlib
from matplotlib.cbook import is_string_like, iterable
Expand Down Expand Up @@ -164,3 +166,21 @@ def setup():
rcdefaults() # Start with all defaults

set_font_settings_for_testing()


@contextmanager
def closed_tempfile(suffix='', text=None):
"""
Context manager which yields the path to a closed temporary file with the
Copy link
Member
@NelleV NelleV Sep 30, 2016

Choose a reason for hiding this comment

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

Can you please rewrite this docstring with the numpydoc format? I find the docstring relatively unclear on what exactly the function does. If I understand correctly, it creates, closes and returns a path to a file and deletes it on exiting the context. It is also unclear where the file is created. A note on how it relates and differs from tempfile.NamedTemporaryFile would help clarify what the function does.

Also, this is publicly available - do we want this publicly available? (if not, then the docstring comment can be ignored.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does publicly available mean should be used outside of matplotlib, or is the recommended option for other parts of matplotlib? I don't see much use outside matplotlib, unless you were already using matplotlib's testing utils.

suffix `suffix`. The file will be deleted on exiting the context. An
additional argument `text` can be provided to have the file contain `text`.
"""
with tempfile.NamedTemporaryFile(
'w+t', suffix=suffix, delete=False
) as test_file:
Copy link
Member
@NelleV NelleV Sep 30, 2016

Choose a reason for hiding this comment

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

That parenthesis here is weird. I am surprised pep8 allows this. Is there anyway to break the line in another way?

Copy link
Member

Choose a reason for hiding this comment

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

    with tempfile.NamedTemporaryFile('w+t', suffix=suffix,
                                     delete=False) as test_file:

fits within 79 chars, and looks reasonable, I think.

file_name = test_file.name
if text is not None:
test_file.write(text)
test_file.flush()
yield file_name
shutil.rmtree(file_name, ignore_errors=True)
53 changes: 10 additions & 43 deletions lib/matplotlib/tests/test_animation.py
8000
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import matplotlib as mpl
from matplotlib import pyplot as plt
from matplotlib import animation
from ..testing import xfail, skip
from ..testing import xfail, skip, closed_tempfile
from ..testing.decorators import cleanup


Expand Down Expand Up @@ -134,20 +134,9 @@ def animate(i):
line.set_data(x, y)
return line,

# Use NamedTemporaryFile: will be automatically deleted
F = tempfile.NamedTemporaryFile(suffix='.' + extension)
F.close()
anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5)
try:
anim.save(F.name, fps=30, writer=writer, bitrate=500)
except UnicodeDecodeError:
xfail("There can be errors in the numpy import stack, "
"see issues #1891 and #2679")
finally:
try:
os.remove(F.name)
except Exception:
pass
with closed_tempfile(suffix='.' + extension) as fname:
anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5)
anim.save(fname, fps=30, writer=writer, bitrate=500)


@cleanup
Expand Down Expand Up @@ -178,20 +167,9 @@ def animate(i):
line.set_data(x, y)
return line,

# Use NamedTemporaryFile: will be automatically deleted
F = tempfile.NamedTemporaryFile(suffix='.' + extension)
F.close()
anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5)
try:
anim.save(FakeFSPathClass(F.name), fps=30, writer=writer, bitrate=500)
except UnicodeDecodeError:
xfail("There can be errors in the numpy import stack, "
"see issues #1891 and #2679")
finally:
try:
os.remove(F.name)
except Exception:
pass
with closed_tempfile(suffix='.' + extension) as fname:
anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5)
anim.save(FakeFSPathClass(fname), fps=30, writer=writer, bitrate=500)


@cleanup
Expand Down Expand Up @@ -219,20 +197,9 @@ def animate(i):
line.set_data(x, y)
return line,

# Use NamedTemporaryFile: will be automatically deleted
F = tempfile.NamedTemporaryFile(suffix='.' + extension)
F.close()
anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5)
try:
anim.save(Path(F.name), fps=30, writer=writer, bitrate=500)
except UnicodeDecodeError:
xfail("There can be errors in the numpy import stack, "
"see issues #1891 and #2679")
finally:
try:
os.remove(F.name)
except Exception:
pass
with closed_tempfile(suffix='.' + extension) as fname:
anim = animation.FuncAnimation(fig, animate, init_func=init, frames=5)
anim.save(Path(fname), fps=30, writer=writer, bitrate=500)


@cleanup
Expand Down
37 changes: 14 additions & 23 deletions lib/matplotlib/tests/test_backend_pdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from matplotlib import pyplot as plt
from matplotlib.testing.decorators import (image_comparison, knownfailureif,
cleanup)
from matplotlib.testing import skip
from matplotlib.testing import skip, closed_tempfile

if 'TRAVIS' not in os.environ:
@image_comparison(baseline_images=['pdf_use14corefonts'],
Expand Down Expand Up @@ -57,14 +57,12 @@ def test_multipage_pagecount():
@cleanup
def test_multipage_keep_empty():
from matplotlib.backends.backend_pdf import PdfPages
from tempfile import NamedTemporaryFile
# test empty pdf files
# test that an empty pdf is left behind with keep_empty=True (default)
with NamedTemporaryFile(delete=False) as tmp:
with closed_tempfile(".pdf") as tmp:
with PdfPages(tmp) as pdf:
filename = pdf._file.fh.name
assert os.path.exists(filename)
os.remove(filename)
# test if an empty pdf is deleting itself afterwards with keep_empty=False
with PdfPages(filename, keep_empty=False) as pdf:
pass
Expand All @@ -74,19 +72,17 @@ def test_multipage_keep_empty():
ax = fig.add_subplot(111)
ax.plot([1, 2, 3])
# test that a non-empty pdf is left behind with keep_empty=True (default)
with NamedTemporaryFile(delete=False) as tmp:
with closed_tempfile(".pdf") as tmp:
with PdfPages(tmp) as pdf:
filename = pdf._file.fh.name
pdf.savefig()
assert os.path.exists(filename)
os.remove(filename)
# test that a non-empty pdf is left behind with keep_empty=False
with NamedTemporaryFile(delete=False) as tmp:
with closed_tempfile(".pdf") as tmp:
with PdfPages(tmp, keep_empty=False) as pdf:
filename = pdf._file.fh.name
pdf.savefig()
assert os.path.exists(filename)
os.remove(filename)


@cleanup
Expand Down Expand Up @@ -137,20 +133,17 @@ def test_grayscale_alpha():

@cleanup
def test_pdfpages_accept_pep_519():
Copy link
Member

Choose a reason for hiding this comment

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

Those two tests look like they could be refactored into one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test_pdfpages_accept_pep_519 and test_savefig_accept_pathlib? I'm not sure it's possible to skip part of a test and have that appear in nose/pytest.

Copy link
Member

Choose a reason for hiding this comment

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

mmh… good question.
@Kojoley any idea on that?

from tempfile import NamedTemporaryFile

class FakeFSPathClass(object):
def __init__(self, path):
self._path = path

def __fspath__(self):
return self._path
tmpfile = NamedTemporaryFile(suffix='.pdf')
tmpfile.close()
with PdfPages(FakeFSPathClass(tmpfile.name)) as pdf:
fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
pdf.savefig(fig)
with closed_tempfile(suffix='.pdf') as fname:
with PdfPages(FakeFSPathClass(fname)) as pdf:
fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
pdf.savefig(fig)
Copy link
Contributor

Choose a reason for hiding this comment

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

... but this doesn't: the NamedTemporaryFile still has the file open and savefig writes to it. The other test closes the file and reuses only the name.

Copy link
Contributor

Choose a reason for hiding this comment

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

in pypandoc I use this:

@contextlib.contextmanager
def closed_tempfile(suffix, text=None):
    with tempfile.NamedTemporaryFile('w+t', suffix=suffix, delete=False) as test_file:
        file_name = test_file.name
        if text:
            test_file.write(text)
            test_file.flush()
    yield file_name
    shutil.rmtree(file_name, ignore_errors=True)

[...]

with closed_tempfile('.md', text='#some title\n') as file_name:
    # do something...



@cleanup
Expand All @@ -159,13 +152,11 @@ def test_savefig_accept_pathlib():
from pathlib import Path
except ImportError:
skip("pathlib not installed")
from tempfile import NamedTemporaryFile

fig, ax = plt.subplots()
C3FE ax.plot([1, 2], [3, 4])
tmpfile = NamedTemporaryFile(suffix='.pdf')
tmpfile.close()
with PdfPages(Path(tmpfile.name)) as pdf:
fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
pdf.savefig(fig)
with closed_tempfile(suffix='.pdf') as fname:
with PdfPages(Path(fname)) as pdf:
fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
pdf.savefig(fig)
19 changes: 7 additions & 12 deletions lib/matplotlib/tests/test_cbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from nose.tools import (assert_equal, assert_not_equal, raises, assert_true,
assert_raises)
from matplotlib.testing.decorators import cleanup
from matplotlib.testing import skip
from matplotlib.testing import skip, closed_tempfile

import matplotlib.cbook as cbook
import matplotlib.colors as mcolors
Expand Down Expand Up @@ -529,19 +529,16 @@ def test_flatiter():

@cleanup
def test_to_filehandle_accept_pep_519():
from tempfile import NamedTemporaryFile

class FakeFSPathClass(object):
def __init__(self, path):
self._path = path

def __fspath__(self):
return self._path

tmpfile = NamedTemporaryFile(delete=False)
tmpfile.close()
pep519_path = FakeFSPathClass(tmpfile.name)
cbook.to_filehandle(pep519_path)
with closed_tempfile() as tmpfile:
pep519_path = FakeFSPathClass(tmpfile)
cbook.to_filehandle(pep519_path)


@cleanup
Expand All @@ -550,9 +547,7 @@ def test_to_filehandle_accept_pathlib():
from pathlib import Path
except ImportError:
skip("pathlib not installed")
from tempfile import NamedTemporaryFile

tmpfile = NamedTemporaryFile(delete=False)
tmpfile.close()
path = Path(tmpfile.name)
cbook.to_filehandle(path)
with closed_tempfile() as tmpfile:
path = Path(tmpfile)
cbook.to_filehandle(path)
51 changes: 19 additions & 32 deletions lib/matplotlib/tests/test_figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from matplotlib import rcParams
from matplotlib.testing.decorators import image_comparison, cleanup
from matplotlib.testing import skip
from matplotlib.testing import skip, closed_tempfile
from matplotlib.axes import Axes
import matplotlib.pyplot as plt
import numpy as np
Expand Down Expand Up @@ -209,8 +209,6 @@ def test_figaspect():

@cleanup
def test_savefig_accept_pep_519_png():
from tempfile import NamedTemporaryFile

class FakeFSPathClass(object):
def __init__(self, path):
self._path = path
Expand All @@ -221,10 +219,9 @@ def __fspath__(self):
fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])

tmpfile = NamedTemporaryFile(suffix='.png')
tmpfile.close()
pep519_path = FakeFSPathClass(tmpfile.name)
fig.savefig(pep519_path)
with closed_tempfile(suffix='.png') as fname:
pep519_path = FakeFSPathClass(fname)
fig.savefig(pep519_path)


@cleanup
Expand All @@ -233,19 +230,16 @@ def test_savefig_accept_pathlib_png():
from pathlib import Path
except ImportError:
skip("pathlib not installed")
from tempfile import NamedTemporaryFile

fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
tmpfile = NamedTemporaryFile(suffix='.png')
tmpfile.close()
path = Path(tmpfile.name)
fig.savefig(path)
with closed_tempfile(suffix='.png') as fname:
path = Path(fname)
fig.savefig(path)


@cleanup
def test_savefig_accept_pep_519_svg():
from tempfile import NamedTemporaryFile

class FakeFSPathClass(object):
def __init__(self, path):
Expand All @@ -256,10 +250,9 @@ def __fspath__(self):

fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
tmpfile = NamedTemporaryFile(suffix='.svg')
tmpfile.close()
pep519_path = FakeFSPathClass(tmpfile.name)
fig.savefig(pep519_path)
with closed_tempfile(suffix='.svg') as fname:
pep519_path = FakeFSPathClass(fname)
fig.savefig(pep519_path)


@cleanup
Expand All @@ -268,19 +261,16 @@ def test_savefig_accept_pathlib_svg():
from pathlib import Path
except ImportError:
skip("pathlib not installed")
from tempfile import NamedTemporaryFile

fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
tmpfile = NamedTemporaryFile(suffix='.svg')
tmpfile.close()
path = Path(tmpfile.name)
fig.savefig(path)
with closed_tempfile(suffix='.svg') as fname:
path = Path(fname)
fig.savefig(path)


@cleanup
def test_savefig_accept_pep_519_pdf():
from tempfile import NamedTemporaryFile

class FakeFSPathClass(object):
def __init__(self, path):
Expand All @@ -291,10 +281,9 @@ def __fspath__(self):

fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
tmpfile = NamedTemporaryFile(suffix='.pdf')
tmpfile.close()
pep519_path = FakeFSPathClass(tmpfile.name)
fig.savefig(pep519_path)
with closed_tempfile(suffix='.pdf') as fname:
pep519_path = FakeFSPathClass(fname)
fig.savefig(pep519_path)


@cleanup
Expand All @@ -303,14 +292,12 @@ def test_savefig_accept_pathlib_pdf():
from pathlib import Path
except ImportError:
skip("pathlib not installed")
from tempfile import NamedTemporaryFile

fig, ax = plt.subplots()
ax.plot([1, 2], [3, 4])
tmpfile = NamedTemporaryFile(suffix='.pdf')
tmpfile.close()
path = Path(tmpfile.name)
fig.savefig(path)
with closed_tempfile(suffix='.pdf') as fname:
path = Path(fname)
fig.savefig(path)

if __name__ == "__main__":
import nose
Expand Down
13 changes: 4 additions & 9 deletions lib/matplotlib/tests/test_font_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from matplotlib.font_manager import (
findfont, FontProperties, fontManager, json_dump, json_load, get_font,
is_opentype_cff_font, fontManager as fm)
from matplotlib.testing import closed_tempfile
import os.path


Expand All @@ -36,15 +37,9 @@ def test_font_priority():
def test_json_serialization():
# on windows, we can't open a file twice, so save the name and unlink
# manually...
try:
name = None
with tempfile.NamedTemporaryFile(delete=False) as temp:
name = temp.name
json_dump(fontManager, name)
copy = json_load(name)
finally:
if name and os.path.exists(name):
os.remove(name)
with closed_tempfile(".json") as temp:
json_dump(fontManager, temp)
copy = json_load(temp)
with warnings.catch_warnings():
warnings.filterwarnings('ignore', 'findfont: Font family.*not found')
for prop in ({'family': 'STIXGeneral'},
Expand Down
Loading
0