Make more requested changes for PR 3236

- Moved several variable assignments outside of try blocks
- Added and clarified various comments and docstrings
- Modified the command loop in `get_im_version()` to a slightly more
readable approach
- `get_im_version()` now returns None when ImageMagick is unavailable
- Updated `ArtResizer._check_method` to handle our new returns in a way
that is more readable
- Fixed an issue where `get_im_version()` could crash if the regex
search failed to find a match
This commit is contained in:
ababyduck 2019-04-25 07:50:12 -07:00
parent 09abd9802a
commit 278d87f25a

View file

@ -82,7 +82,7 @@ def pil_resize(maxwidth, path_in, path_out=None):
def im_resize(maxwidth, path_in, path_out=None):
"""Resize using ImageMagick's ``magick`` tool
(or fall back to ``convert`` for older versions.)
(or fall back to ``convert`` for older versions).
Return the output path of resized image.
"""
path_out = path_out or temp_file_for(path_in)
@ -92,17 +92,18 @@ def im_resize(maxwidth, path_in, path_out=None):
# "-resize WIDTHx>" shrinks images with the width larger
# than the given width while maintaining the aspect ratio
# with regards to the height.
try:
cmd = ArtResizer.shared.im_convert_cmd + \
[util.syspath(path_in, prefix=False),
'-resize', '{0}x>'.format(maxwidth),
util.syspath(path_out, prefix=False)]
cmd = ArtResizer.shared.im_convert_cmd + \
[util.syspath(path_in, prefix=False),
'-resize', '{0}x>'.format(maxwidth),
util.syspath(path_out, prefix=False)]
try:
util.command_output(cmd)
except subprocess.CalledProcessError:
log.warning(u'artresizer: IM convert failed for {0}',
util.displayable_path(path_in))
return path_in
return path_out
@ -123,10 +124,10 @@ def pil_getsize(path_in):
def im_getsize(path_in):
try:
cmd = ArtResizer.shared.im_identify_cmd + \
['-format', '%w %h', util.syspath(path_in, prefix=False)]
cmd = ArtResizer.shared.im_identify_cmd + \
['-format', '%w %h', util.syspath(path_in, prefix=False)]
try:
out = util.command_output(cmd)
except subprocess.CalledProcessError as exc:
log.warning(u'ImageMagick size query failed')
@ -176,6 +177,9 @@ class ArtResizer(six.with_metaclass(Shareable, object)):
log.debug(u"artresizer: method is {0}", self.method)
self.can_compare = self._can_compare()
# Use ImageMagick's magick binary when it's available. If it's
# not, fall back to the older, separate convert and identify
# commands.
if self.method[0] == IMAGEMAGICK:
self.im_legacy = self.method[2]
if self.im_legacy:
@ -230,13 +234,14 @@ class ArtResizer(six.with_metaclass(Shareable, object)):
@staticmethod
def _check_method():
"""Return a tuple indicating an available method and its version."""
try:
version, legacy = get_im_version()
if version > (0, 0, 0):
return IMAGEMAGICK, version, legacy
except TypeError:
pass
"""Return a tuple indicating an available method and its version.
If the method is ImageMagick, also return a bool indicating whether to
use the `magick` binary or legacy utils (`convert`, `identify`, etc.)
"""
version = get_im_version()
if version:
version, legacy = version
return IMAGEMAGICK, version, legacy
version = get_pil_version()
if version:
@ -246,40 +251,36 @@ class ArtResizer(six.with_metaclass(Shareable, object)):
def get_im_version():
"""Return Image Magick version or None if it is unavailable
Try invoking ImageMagick's "magick". If "magick" is unavailable,
as with older versions, fall back to "convert"
"""Return ImageMagick version/legacy-flag pair or None if the check fails.
Our iterator will be non-zero when the first command fails, and will
be returned in a tuple along with the version.
Try invoking ImageMagick's `magick` binary first, then `convert` if
`magick` is unavailable.
"""
cmd_names = (['magick'],
['convert'])
for i, cmd_name in enumerate(cmd_names):
for cmd_name, legacy in ((['magick'], False), (['convert'], True)):
cmd = cmd_name + ['--version']
try:
cmd = cmd_name + ['--version']
out = util.command_output(cmd)
if b'imagemagick' in out.lower():
pattern = br".+ (\d+)\.(\d+)\.(\d+).*"
match = re.search(pattern, out)
version = (int(match.group(1)),
int(match.group(2)),
int(match.group(3)))
legacy = bool(i)
if match:
return (version, legacy)
version = (int(match.group(1)),
int(match.group(2)),
int(match.group(3)))
return version, legacy
except (subprocess.CalledProcessError, OSError) as exc:
log.debug(u'ImageMagick version check failed: {}', exc)
return ((0,), None)
return None
def get_pil_version():
"""Return Image Magick version or None if it is unavailable
Try importing PIL."""
"""Return Pillow version or None if it is unavailable
Try importing PIL.
"""
try:
__import__('PIL', fromlist=[str('Image')])
return (0,)