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

@@ -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.
 

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
 

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,

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

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)

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

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):

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

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

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')

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):

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"."):

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)

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()

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')
 

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:

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)

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