From 0e17b32740d1a161c54df419c587a6fc46ccd02c Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Wed, 16 Jan 2019 20:40:55 +0000 Subject: [PATCH] Various stylistic fixes - Fix PEP8's line too long warning - Remove useless parentheses - Use triple quotes for docstring - Address pylint's 'inconsistent-return-statements' - Specify string format arguments as logging function parameters - Comment out already disabled block of code - Remove useless else (after return) - Remove useless statement - Do not import already imported module --- whipper/command/basecommand.py | 2 +- whipper/command/cd.py | 2 ++ whipper/command/main.py | 10 +++++----- whipper/command/mblookup.py | 2 ++ whipper/command/offset.py | 6 ++++-- whipper/common/cache.py | 5 ++--- whipper/common/common.py | 15 +++++++-------- whipper/common/drive.py | 2 +- whipper/common/mbngs.py | 4 +--- whipper/common/program.py | 8 ++++---- whipper/common/renamer.py | 4 +--- whipper/extern/freedb.py | 2 +- whipper/image/image.py | 1 - whipper/image/table.py | 2 +- whipper/program/cdparanoia.py | 10 ++++------ whipper/program/cdrdao.py | 4 ++-- whipper/program/utils.py | 2 +- whipper/test/common.py | 2 +- 18 files changed, 40 insertions(+), 43 deletions(-) diff --git a/whipper/command/basecommand.py b/whipper/command/basecommand.py index 4a81af2..5ad49f3 100644 --- a/whipper/command/basecommand.py +++ b/whipper/command/basecommand.py @@ -25,7 +25,7 @@ logger = logging.getLogger(__name__) # options) to the child command. -class BaseCommand(): +class BaseCommand: """ Register and handle whipper command arguments with ArgumentParser. diff --git a/whipper/command/cd.py b/whipper/command/cd.py index f8d6824..aed96d8 100644 --- a/whipper/command/cd.py +++ b/whipper/command/cd.py @@ -191,6 +191,8 @@ class _CD(BaseCommand): if self.options.eject in ('success', 'always'): utils.eject_device(self.device) + return None + def doCommand(self): pass diff --git a/whipper/command/main.py b/whipper/command/main.py index adaec7b..651f748 100644 --- a/whipper/command/main.py +++ b/whipper/command/main.py @@ -74,11 +74,11 @@ def main(): class Whipper(BaseCommand): - description = """whipper is a CD ripping utility focusing on accuracy over speed. - -whipper gives you a tree of subcommands to work with. -You can get help on subcommands by using the -h option to the subcommand. -""" + description = ( + "whipper is a CD ripping utility focusing on accuracy over speed.\n\n" + "whipper gives you a tree of subcommands to work with.\n" + "You can get help on subcommands by using the -h option " + "to the subcommand.\n") no_add_help = True subcommands = { 'accurip': accurip.AccuRip, diff --git a/whipper/command/mblookup.py b/whipper/command/mblookup.py index 53f8b19..e8e7c60 100644 --- a/whipper/command/mblookup.py +++ b/whipper/command/mblookup.py @@ -42,3 +42,5 @@ Example disc id: KnpGsLhvH.lPrNc1PBL21lb9Bg4-""" j + 1, track.artist.encode('utf-8'), track.title.encode('utf-8') )) + + return None diff --git a/whipper/command/offset.py b/whipper/command/offset.py index 9fe620c..300d90f 100644 --- a/whipper/command/offset.py +++ b/whipper/command/offset.py @@ -94,7 +94,7 @@ CD in the AccurateRip database.""" except accurip.EntryNotFound: logger.warning("AccurateRip entry not found: drive offset " "can't be determined, try again with another disc") - return + return None if responses: logger.debug('%d AccurateRip responses found.', len(responses)) @@ -170,6 +170,8 @@ CD in the AccurateRip database.""" logger.error('no matching offset found. ' 'Consider trying again with a different disc') + return None + def _arcs(self, runner, table, track, offset): # rips the track with the given offset, return the arcs checksums logger.debug('ripping track %r with offset %d...', track, offset) @@ -196,7 +198,7 @@ CD in the AccurateRip database.""" ) os.unlink(path) - return ("%08x" % v1, "%08x" % v2) + return "%08x" % v1, "%08x" % v2 def _foundOffset(self, device, offset): print('\nRead offset of device is: %d.' % offset) diff --git a/whipper/common/cache.py b/whipper/common/cache.py index 5305ed3..18fe6b3 100644 --- a/whipper/common/cache.py +++ b/whipper/common/cache.py @@ -93,10 +93,10 @@ class Persister: self.object = default if not self._path: - return None + return if not os.path.exists(self._path): - return None + return handle = open(self._path) import pickle @@ -109,7 +109,6 @@ class Persister: # can fail for various reasons; in that case, pretend we didn't # load it logger.debug(e) - pass def delete(self): self.object = None diff --git a/whipper/common/common.py b/whipper/common/common.py index 675b021..b81395b 100644 --- a/whipper/common/common.py +++ b/whipper/common/common.py @@ -116,7 +116,7 @@ def formatTime(seconds, fractional=3): chunks = [] if seconds < 0: - chunks.append(('-')) + chunks.append('-') seconds = -seconds hour = 60 * 60 @@ -271,13 +271,12 @@ def getRelativePath(targetPath, collectionPath): if targetDir == collectionDir: logger.debug('getRelativePath: target and collection in same dir') return os.path.basename(targetPath) - else: - rel = os.path.relpath( - targetDir + os.path.sep, - collectionDir + os.path.sep) - logger.debug('getRelativePath: target and collection ' - 'in different dir, %r', rel) - return os.path.join(rel, os.path.basename(targetPath)) + rel = os.path.relpath( + targetDir + os.path.sep, + collectionDir + os.path.sep) + logger.debug('getRelativePath: target and collection ' + 'in different dir, %r', rel) + return os.path.join(rel, os.path.basename(targetPath)) def validate_template(template, kind): diff --git a/whipper/common/drive.py b/whipper/common/drive.py index 37950fd..226e04d 100644 --- a/whipper/common/drive.py +++ b/whipper/common/drive.py @@ -68,4 +68,4 @@ def getDeviceInfo(path): device = cdio.Device(path) ok, vendor, model, release = device.get_hwinfo() - return (vendor, model, release) + return vendor, model, release diff --git a/whipper/common/mbngs.py b/whipper/common/mbngs.py index aaab4c1..c16d529 100644 --- a/whipper/common/mbngs.py +++ b/whipper/common/mbngs.py @@ -317,6 +317,4 @@ def musicbrainz(discid, country=None, record=False): return ret elif result.get('cdstub'): logger.debug('query returned cdstub: ignored') - return None - else: - return None + return None diff --git a/whipper/common/program.py b/whipper/common/program.py index ad468a2..383ffcc 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -141,7 +141,7 @@ class Program: return self.result def addDisambiguation(self, template_part, metadata): - "Add disambiguation to template path part string." + """Add disambiguation to template path part string.""" if metadata.catalogNumber: template_part += ' (%s)' % metadata.catalogNumber elif metadata.barcode: @@ -327,7 +327,7 @@ class Program: elif not metadatas: logger.warning("requested release id '%s', but none of " "the found releases match", release) - return + return None else: if lowest: metadatas = deltas[lowest] @@ -346,7 +346,7 @@ class Program: "not the same", releaseTitle, i, metadata.releaseTitle) - if (not release and len(list(deltas)) > 1): + if not release and len(list(deltas)) > 1: logger.warning('picked closest match in duration. ' 'Others may be wrong in MusicBrainz, ' 'please correct') @@ -438,7 +438,7 @@ class Program: start = index.absolute stop = track.getIndex(1).absolute - 1 - return (start, stop) + return start, stop def verifyTrack(self, runner, trackResult): is_wave = not trackResult.filename.endswith('.flac') diff --git a/whipper/common/renamer.py b/whipper/common/renamer.py index 8db09a4..0f24c36 100644 --- a/whipper/common/renamer.py +++ b/whipper/common/renamer.py @@ -21,9 +21,7 @@ import os import tempfile -""" -Rename files on file system and inside metafiles in a resumable way. -""" +"""Rename files on file system and inside metafiles in a resumable way.""" class Operator(object): diff --git a/whipper/extern/freedb.py b/whipper/extern/freedb.py index 48a3deb..d08fcb7 100644 --- a/whipper/extern/freedb.py +++ b/whipper/extern/freedb.py @@ -145,7 +145,7 @@ def perform_lookup(disc_id, freedb_server, freedb_port): response = RESPONSE.match(next(query)) if response is not None: - # FIXME - check response code here + # FIXME: check response code here freedb = {} line = next(query) while not line.startswith(u"."): diff --git a/whipper/image/image.py b/whipper/image/image.py index 93120cc..0d05893 100644 --- a/whipper/image/image.py +++ b/whipper/image/image.py @@ -205,7 +205,6 @@ class ImageEncodeTask(task.MultiSeparateTask): add(htoa) except (KeyError, IndexError): logger.debug('no HTOA track') - pass for trackIndex, track in enumerate(cue.table.tracks): logger.debug('encoding track %d', trackIndex + 1) diff --git a/whipper/image/table.py b/whipper/image/table.py index 1d03f96..64050ab 100644 --- a/whipper/image/table.py +++ b/whipper/image/table.py @@ -824,7 +824,7 @@ class Table(object): discId1 &= 0xffffffff discId2 &= 0xffffffff - return ("%08x" % discId1, "%08x" % discId2) + return "%08x" % discId1, "%08x" % discId2 def accuraterip_path(self): discId1, discId2 = self.accuraterip_ids() diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index 74871a4..6fe475a 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -159,11 +159,10 @@ class ProgressParser: markEnd = frameOffset # FIXME: doing this is way too slow even for a testcase, so disable - if False: - for frame in range(markStart, markEnd): - if frame not in list(self._reads.keys()): - self._reads[frame] = 0 - self._reads[frame] += 1 + # for frame in range(markStart, markEnd): + # if frame not in list(self._reads.keys()): + # self._reads[frame] = 0 + # self._reads[frame] += 1 # cdparanoia reads quite a bit beyond the current track before it # goes back to verify; don't count those @@ -300,7 +299,6 @@ class ReadTrackTask(task.Task): stderr=subprocess.PIPE, close_fds=True) except OSError as e: - import errno if e.errno == errno.ENOENT: raise common.MissingDependencyException('cd-paranoia') diff --git a/whipper/program/cdrdao.py b/whipper/program/cdrdao.py index acaebd9..36a0f89 100644 --- a/whipper/program/cdrdao.py +++ b/whipper/program/cdrdao.py @@ -125,8 +125,8 @@ class ReadTOCTask(task.Task): self._buffer = "" for line in lines: self._parser.parse(line) - if (self._parser.currentTrack is not 0 and - self._parser.tracks is not 0): + if (self._parser.currentTrack != 0 and + self._parser.tracks != 0): progress = (float('%d' % self._parser.currentTrack) / float(self._parser.tracks)) if progress < 1.0: diff --git a/whipper/program/utils.py b/whipper/program/utils.py index f332738..e4133a6 100644 --- a/whipper/program/utils.py +++ b/whipper/program/utils.py @@ -28,7 +28,7 @@ def unmount_device(device): If the given device is a symlink, the target will be checked. """ device = os.path.realpath(device) - logger.debug('possibly unmount real path %r' % device) + logger.debug('possibly unmount real path %r', device) proc = open('/proc/mounts').read() if device in proc: print('Device %s is mounted, unmounting' % device) diff --git a/whipper/test/common.py b/whipper/test/common.py index ad2c084..49cf109 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -71,7 +71,7 @@ class TestCase(unittest.TestCase): ret = open(cuefile).read().decode('utf-8') ret = re.sub( 'REM COMMENT "whipper.*', - 'REM COMMENT "whipper %s"' % (whipper.__version__), + 'REM COMMENT "whipper %s"' % whipper.__version__, ret, re.MULTILINE) return ret