Skip to content

Small cleanups. #12899

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

Merged
merged 1 commit into from
Mar 3, 2019
Merged

Small cleanups. #12899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 11 additions & 35 deletions lib/matplotlib/backends/backend_pgf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
_Backend, FigureCanvasBase, FigureManagerBase, GraphicsContextBase,
RendererBase)
from matplotlib.backends.backend_mixed import MixedModeRenderer
from matplotlib.cbook import is_writable_file_like
from matplotlib.path import Path
from matplotlib.figure import Figure
from matplotlib._pylab_helpers import Gcf
Expand Down Expand Up @@ -385,8 +384,8 @@ def __init__(self, figure, fh, dummy=False):
Matplotlib figure to initialize height, width and dpi from.
fh : file-like
File handle for the output of the drawing commands.

"""

RendererBase.__init__(self)
self.dpi = figure.dpi
self.fh = fh
Expand Down Expand Up @@ -842,16 +841,10 @@ def print_pgf(self, fname_or_fh, *args, **kwargs):
if kwargs.get("dryrun", False):
self._print_pgf_to_fh(None, *args, **kwargs)
return

# figure out where the pgf is to be written to
if isinstance(fname_or_fh, str):
with open(fname_or_fh, "w", encoding="utf-8") as fh:
self._print_pgf_to_fh(fh, *args, **kwargs)
elif is_writable_file_like(fname_or_fh):
fh = codecs.getwriter("utf-8")(fname_or_fh)
self._print_pgf_to_fh(fh, *args, **kwargs)
else:
raise ValueError("filename must be a path")
with cbook.open_file_cm(fname_or_fh, "w", encoding="utf-8") as file:
if not cbook.file_requires_unicode(file):
file = codecs.getwriter("utf-8")(file)
Copy link
Member

Choose a reason for hiding this comment

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

Can we not clobber file like this, please?

Copy link
Member

Choose a reason for hiding this comment

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

Also, when reassigning a variable within a context like this, I don't think the close() gets called on the new object because the context manager is holding a reference to the original object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clobbering what? file is not a builtin (... anymore). And I'd say it's a much nicer variable name than fh...

The fact that the codec writer doesn't get closed doesn't matter (and anyways it will get closed by the GC), what matters is that the file itself gets closed.

import codecs
import contextlib


class filelike:  # mocks file
    def close(self):
        print("closed")


@contextlib.contextmanager
def cm():  # mocks open_file_cm
    print("enter")
    f = filelike()
    yield f
    f.close()
    print("exit")


with cm() as file:
    file = codecs.getwriter("utf-8")(file)


print("done")

prints

enter
closed
exit
done

so the file is indeed closed before the contextmanager is exited.

self._print_pgf_to_fh(file, *args, **kwargs)

def _print_pdf_to_fh(self, fh, *args, **kwargs):
w, h = self.figure.get_figwidth(), self.figure.get_figheight()
Expand Down Expand Up @@ -896,21 +889,12 @@ def _print_pdf_to_fh(self, fh, *args, **kwargs):
TmpDirCleaner.add(tmpdir)

def print_pdf(self, fname_or_fh, *args, **kwargs):
"""
Use LaTeX to compile a Pgf generated figure to PDF.
"""
"""Use LaTeX to compile a Pgf generated figure to PDF."""
if kwargs.get("dryrun", False):
self._print_pgf_to_fh(None, *args, **kwargs)
return

# figure out where the pdf is to be written to
if isinstance(fname_or_fh, str):
with open(fname_or_fh, "wb") as fh:
self._print_pdf_to_fh(fh, *args, **kwargs)
elif is_writable_file_like(fname_or_fh):
self._print_pdf_to_fh(fname_or_fh, *args, **kwargs)
else:
raise ValueError("filename must be a path or a file-like object")
with cbook.open_file_cm(fname_or_fh, "wb") as file:
self._print_pdf_to_fh(file, *args, **kwargs)

def _print_png_to_fh(self, fh, *args, **kwargs):
converter = make_pdf_to_png_converter()
Expand All @@ -933,20 +917,12 @@ def _print_png_to_fh(self, fh, *args, **kwargs):
TmpDirCleaner.add(tmpdir)

def print_png(self, fname_or_fh, *args, **kwargs):
"""
Use LaTeX to compile a pgf figure to pdf and convert it to png.
"""
"""Use LaTeX to compile a pgf figure to pdf and convert it to png."""
if kwargs.get("dryrun", False):
self._print_pgf_to_fh(None, *args, **kwargs)
return

if isinstance(fname_or_fh, str):
with open(fname_or_fh, "wb") as fh:
self._print_png_to_fh(fh, *args, **kwargs)
elif is_writable_file_like(fname_or_fh):
self._print_png_to_fh(fname_or_fh, *args, **kwargs)
else:
raise ValueError("filename must be a path or a file-like object")
with cbook.open_file_cm(fname_or_fh, "wb") as file:
self._print_png_to_fh(file, *args, **kwargs)

def get_renderer(self):
return RendererPgf(self.figure, None, dummy=True)
Expand Down
6 changes: 3 additions & 3 deletions lib/matplotlib/cbook/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,14 @@ def is_hashable(obj):


def is_writable_file_like(obj):
"""return true if *obj* looks like a file object with a *write* method"""
"""Return whether *obj* looks like a file object with a *write* method."""
return callable(getattr(obj, 'write', None))


def file_requires_unicode(x):
"""
Returns `True` if the given writable file-like object requires Unicode
to be written to it.
Returns whether the given writable file-like object requires Unicode to be
written to it.
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I like the original wording a bit better because it states what is returned, rather than implying a boolean result via the word "whether", which might be confusing for non-english-native readers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A quick grep shows that there's a lot of places where we're using Return whether to indicate a boolean return type (though admittedly I may be responsible for quite a few of them, as I also prefer that wording). It's also quite common in the CPython docs (for example).

"""
try:
x.write(b'')
Expand Down
11 changes: 5 additions & 6 deletions lib/matplotlib/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,9 +730,7 @@ def set_interpolation(self, s):
self.stale = True

def can_composite(self):
"""
Returns `True` if the image can be composited with its neighbors.
"""
"""Return whether the image can be composited with its neighbors."""
trans = self.get_transform()
return (
self._interpolation != 'none' and
Expand All @@ -741,19 +739,20 @@ def can_composite(self):

def set_resample(self, v):
"""
Set whether or not image resampling is used.
Set whether image resampling is used.

Parameters
----------
v : bool
v : bool or None
If None, use :rc:`image.resample` = True.
"""
if v is None:
v = rcParams['image.resample']
self._resample = v
self.stale = True

def get_resample(self):
"""Return the image resample boolean."""
"""Return whether image resampling is used."""
return self._resample

def set_filternorm(self, filternorm):
Expand Down