8000 [MRG+2] MAINT: Refactor the converted-image cache by jkseppan · Pull Request #7764 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

[MRG+2] MAINT: Refactor the converted-image cache #7764

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.

Sign up for GitHub

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 19 commits into from
Closed

[MRG+2] MAINT: Refactor the converted-image cache #7764

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
1b24a16
Factor the converted-image cache out of compare.py
jkseppan Jan 7, 2017
650ddf2
Import cbook again
jkseppan Jan 21, 2017
3b5f636
Lock the cache directory
jkseppan Jan 21, 2017
556fd82
Communicate node-specific cache reports to master if using xdist
jkseppan Jan 22, 2017
9f5a06d
Delete the nose plugin
jkseppan Jan 25, 2017
33f2528
Reinstate deleted functions
jkseppan Jan 25, 2017
2136a26
Make conversion_cache private
jkseppan Jan 25, 2017
6995a27
Make globals private
jkseppan Jan 25, 2017
9ea66dd
Improve wording
jkseppan Jan 25, 2017
3751f01
mkdirs throws OSError
jkseppan Jan 25, 2017
a70081e
Improve documentation in response to review
jkseppan Jan 30, 2017
85f54de
Use pytest facilities
jkseppan Jan 30, 2017
0d51e03
Simplify gs and inkscape version checking logic
jkseppan Jan 30, 2017
9de06c0
pep8 fixes
jkseppan Jan 30, 2017
6007b37
There is no nose plugin any more
jkseppan Feb 18, 2017
e52cdd2
Import the pytest hooks in the other conftest.py files
jkseppan Feb 20, 2017
e5184bb
Omit unnecessary tmpdir deletions
jkseppan Feb 22, 2017
f47cbe4
Move pytest_addoption call to top-level conftest.py
jkseppan Feb 22, 2017
996f22b
pep8 fix
jkseppan Feb 22, 2017
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
Next Next commit
Make globals private
  • Loading branch information
@jkseppan
jkseppan committed Feb 20, 2017
commit 6995a27107e4f9ff6cff509d4ac398359ad8dccc
15 changes: 8 additions & 7 deletions lib/matplotlib/testing/_conversion_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from matplotlib import checkdep_inkscape


class ConversionCache(object):
class _ConversionCache(object):
"""A cache that stores png files converted from svg or pdf formats.

The image comparison test cases compare svg and pdf files by
Expand Down Expand Up @@ -180,7 +180,7 @@ def expire(self):
def get_cache_dir():
cachedir = _get_cachedir()
if cachedir is None:
raise CacheError('No suitable configuration directory')
raise _CacheError('No suitable configuration directory')
cachedir = os.path.join(cachedir, 'test_cache')
return cachedir

Expand All @@ -189,13 +189,14 @@ def ensure_cache_dir(self):
try:
cbook.mkdirs(self.cachedir)
except IOError as e:
raise CacheError("Error creating cache directory %s: %s"
% (self.cachedir, str(e)))
raise _CacheError("Error creating cache directory %s: %s"
% (self.cachedir, str(e)))
if not os.access(self.cachedir, os.W_OK):
raise CacheError("Cache directory %s not writable" % self.cachedir)
raise _CacheError("Cache directory %s not writable"
% self.cachedir)


class CacheError(Exception):
class _CacheError(Exception):
def __init__(self, message):
self.message = message

Expand All @@ -204,4 +205,4 @@ def __str__(self):


# A global cache instance, set by the appropriate test runner.
conversion_cache = None
_conversion_cache = None
12 changes: 6 additions & 6 deletions lib/matplotlib/testing/compare.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ def comparable_formats():
return ['png'] + list(converter)


@cbook.deprecated('2.1', addendum='Use ConversionCache instead')
@cbook.deprecated('2.1', addendum='Use _ConversionCache instead')
Copy link
Member

Choose a reason for hiding this comment

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

Should we advocate for the use of a private class/function or just inform the user of its disappearance?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, removed the addenda.

def get_cache_dir():
return ccache.ConversionCache.get_cache_dir()
return ccache._ConversionCache.get_cache_dir()


@cbook.deprecated('2.1', addendum='Use ConversionCache instead')
@cbook.deprecated('2.1', addendum='Use _ConversionCache instead')
def get_file_hash(path, block_size=2 ** 20):
if path.endswith('.pdf'):
from matplotlib import checkdep_ghostscript
Expand All @@ -136,7 +136,7 @@ def get_file_hash(path, block_size=2 ** 20):
version_tag = checkdep_inkscape().encode('utf-8')
else:
version_tag = None
return ccache.ConversionCache._get_file_hash_static(
return ccache._ConversionCache._get_file_hash_static(
path, block_size, version_tag)


Expand All @@ -148,7 +148,7 @@ def convert(filename, cache=None):
Parameters
----------
filename : str
cache : ConversionCache, optional
cache : _ConversionCache, optional

Returns
-------
Expand Down Expand Up @@ -263,7 +263,7 @@ def compare_images(expected, actual, tol, in_decorator=False, cache=None):
in_decorator : bool
If called from image_comparison decorator, this should be
True. (default=False)
cache : cache.ConversionCache, optional
cache : matplotlib.testing._conversion_cache._ConversionCache, optional

Example
-------
Expand Down
13 changes: 7 additions & 6 deletions lib/matplotlib/testing/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,23 @@ def pytest_configure(config):

max_size = config.getoption('--conversion-cache-max-size')
if max_size is not None:
ccache.conversion_cache = \
ccache.ConversionCache(max_size=int(max_size))
ccache._conversion_cache = \
ccache._ConversionCache(max_size=int(max_size))
else:
ccache.conversion_cache = ccache.ConversionCache()
ccache._conversion_cache = ccache._ConversionCache()
if config.pluginmanager.hasplugin('xdist'):
config.pluginmanager.register(DeferPlugin())


def pytest_unconfigure(config):
ccache.conversion_cache.expire()
ccache._conversion_cache.expire()
matplotlib._called_from_pytest = False


def pytest_sessionfinish(session):
if hasattr(session.config, 'slaveoutput'):
session.config.slaveoutput['cache-report'] = ccache.conversion_cache.report()
session.config.slaveoutput['cache-report'] = \
ccache._conversion_cache.report()


def pytest_terminal_summary(terminalreporter):
Expand All @@ -50,7 +51,7 @@ def pytest_terminal_summary(terminalreporter):
'gets': reduce(lambda x, y: x.union(y),
(rep['gets'] for rep in reports))}
else:
data = ccache.conversion_cache.report()
data = ccache._conversion_cache.report()
tr.write_sep('=', 'Image conversion cache report')
tr.write_line('Hit rate: %d/%d' % (len(data['hits']), len(data['gets'])))
if tr.config.getoption('--conversion-cache-report-misses'):
Expand Down
14 changes: 7 additions & 7 deletions lib/matplotlib/tests/test_cache.py
View file 6DB6
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from nose.tools import raises
Copy link
Member

Choose a reason for hiding this comment

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

Remove nose.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


from matplotlib import cbook
from matplotlib.testing.conversion_cache import ConversionCache, CacheError
from matplotlib.testing._conversion_cache import _ConversionCache, _CacheError


def test_cache_basic():
Expand All @@ -22,7 +22,7 @@ def test_cache_basic():
def intmp(f):
return os.path.join(tmpdir, f)
try:
cache = ConversionCache(intmp('cache'))
cache = _ConversionCache(intmp('cache'))
with open(intmp('fake.pdf'), 'w') as pdf:
pdf.write('this is a fake pdf file')
with open(intmp('fake.svg'), 'w') as svg:
Expand Down Expand Up @@ -63,7 +63,7 @@ def test_cache_expire():
def intmp(*f):
return os.path.join(tmpdir, *f)
try:
cache = ConversionCache(intmp('cache'), 10)
cache = _ConversionCache(intmp('cache'), 10)
for i in range(5):
filename = intmp('cache', 'file%d.png' % i)
with open(filename, 'w') as f:
Expand Down Expand Up @@ -95,9 +95,9 @@ def intmp(*f):

def test_cache_default_dir():
try:
path = ConversionCache.get_cache_dir()
path = _ConversionCache.get_cache_dir()
assert path.endswith('test_cache')
except CacheError:
except _CacheError:
pass


Expand All @@ -107,7 +107,7 @@ def test_cache_default_dir():
def test_cache_mkdir_error(mkdirs):
tmpdir = tempfile.mkdtemp()
Copy link
Member

Choose a reason for hiding this comment

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

Use tmpdir as a parameter to use the pytest fixture.

try:
c = ConversionCache(os.path.join(tmpdir, 'cache'))
c = _ConversionCache(os.path.join(tmpdir, 'cache'))
finally:
shutil.rmtree(tmpdir)

Expand All @@ -120,6 +120,6 @@ def test_cache_unwritable_error(access):
cachedir = os.path.join(tmpdir, 'test_cache')
try:
cbook.mkdirs(cachedir)
c = ConversionCache(cachedir)
c = _ConversionCache(cachedir)
finally:
shutil.rmtree(tmpdir)
0