From 8dfcc5b5ec28263fc7a7555acb1c2900091161f3 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Wed, 16 Jan 2019 19:57:32 +0000 Subject: [PATCH 01/13] Improve regular expressions - Remove redundant escape - Remove erroneous character - Remove duplicate character --- whipper/common/common.py | 4 ++-- whipper/common/path.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/whipper/common/common.py b/whipper/common/common.py index 1b46d4e..675b021 100644 --- a/whipper/common/common.py +++ b/whipper/common/common.py @@ -285,9 +285,9 @@ def validate_template(template, kind): Raise exception if disc/track template includes invalid variables """ if kind == 'disc': - matches = re.findall(r'%[^A,R,S,X,d,r,x,y]', template) + matches = re.findall(r'%[^ARSXdrxy]', template) elif kind == 'track': - matches = re.findall(r'%[^A,R,S,X,a,d,n,r,s,t,x,y]', template) + matches = re.findall(r'%[^ARSXadnrstxy]', template) if '%' in template and matches: raise ValueError(kind + ' template string contains invalid ' 'variable(s): {}'.format(', '.join(matches))) diff --git a/whipper/common/path.py b/whipper/common/path.py index a5d0eb4..268b337 100644 --- a/whipper/common/path.py +++ b/whipper/common/path.py @@ -45,7 +45,7 @@ class PathFilter(object): def separators(path): # replace separators with a space-hyphen or hyphen path = re.sub(r'[:]', ' -', path, re.UNICODE) - path = re.sub(r'[\|]', '-', path, re.UNICODE) + path = re.sub(r'[|]', '-', path, re.UNICODE) return path # change all fancy single/double quotes to normal quotes @@ -56,12 +56,12 @@ class PathFilter(object): if self._special: path = separators(path) - path = re.sub(r'[\*\?&!\'\"\$\(\)`{}\[\]<>]', + path = re.sub(r'[*?&!\'\"$()`{}\[\]<>]', '_', path, re.UNICODE) if self._fat: path = separators(path) # : and | already gone, but leave them here for reference - path = re.sub(r'[:\*\?"<>|"]', '_', path, re.UNICODE) + path = re.sub(r'[:*?"<>|]', '_', path, re.UNICODE) return path From 0e17b32740d1a161c54df419c587a6fc46ccd02c Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Wed, 16 Jan 2019 20:40:55 +0000 Subject: [PATCH 02/13] 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 From fef79731134c5af26f700b49c08bb9e4a58e22c7 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Wed, 16 Jan 2019 21:08:43 +0000 Subject: [PATCH 03/13] Modernize code for easier Python 3 port - Replace print statement with function call - Replace except ..., ...: syntax with except ... as ...: - Simplify obsolete try ... except ... code block - Replace some unicode calls with u-prefixed strings - Do not use `len(SEQUENCE)` to determine if a sequence is empty - Replace dictionary creation - Drop support for GObject static bindings --- misc/offsets.py | 2 +- whipper/command/accurip.py | 4 +--- whipper/extern/asyncsub.py | 8 ++++---- whipper/extern/task/task.py | 13 +++++-------- whipper/image/table.py | 8 ++------ whipper/test/test_common_program.py | 10 ++++------ 6 files changed, 17 insertions(+), 28 deletions(-) diff --git a/misc/offsets.py b/misc/offsets.py index 53b36ea..626f925 100644 --- a/misc/offsets.py +++ b/misc/offsets.py @@ -64,4 +64,4 @@ if len(line) > 11: line = line[:-2] + '"' lines.append(line) -print "\n".join(lines) +print("\n".join(lines)) diff --git a/whipper/command/accurip.py b/whipper/command/accurip.py index e12f6c8..8ea666b 100644 --- a/whipper/command/accurip.py +++ b/whipper/command/accurip.py @@ -59,9 +59,7 @@ retrieves and display accuraterip data from the given URL assert len(r.checksums) == r.num_tracks assert len(r.confidences) == r.num_tracks - entry = {} - entry["confidence"] = r.confidences[track] - entry["response"] = i + 1 + entry = {"confidence": r.confidences[track], "response": i + 1} checksum = r.checksums[track] if checksum in checksums: checksums[checksum].append(entry) diff --git a/whipper/extern/asyncsub.py b/whipper/extern/asyncsub.py index 3ee0fef..76b1e0d 100644 --- a/whipper/extern/asyncsub.py +++ b/whipper/extern/asyncsub.py @@ -53,7 +53,7 @@ class Popen(subprocess.Popen): (errCode, written) = WriteFile(x, input) except ValueError: return self._close('stdin') - except (subprocess.pywintypes.error, Exception), why: + except (subprocess.pywintypes.error, Exception) as why: if why.args[0] in (109, errno.ESHUTDOWN): return self._close('stdin') raise @@ -74,7 +74,7 @@ class Popen(subprocess.Popen): (errCode, read) = ReadFile(x, nAvail, None) except ValueError: return self._close(which) - except (subprocess.pywintypes.error, Exception), why: + except (subprocess.pywintypes.error, Exception) as why: if why.args[0] in (109, errno.ESHUTDOWN): return self._close(which) raise @@ -94,7 +94,7 @@ class Popen(subprocess.Popen): try: written = os.write(self.stdin.fileno(), input) - except OSError, why: + except OSError as why: if why.args[0] == errno.EPIPE: # broken pipe return self._close('stdin') raise @@ -153,7 +153,7 @@ def recv_some(p, t=.1, e=1, tr=5, stderr=0): def send_all(p, data): - while len(data): + while data: sent = p.send(data) if sent is None: raise Exception(message) diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index 250ccd6..c3f86f7 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -22,10 +22,7 @@ from __future__ import print_function import logging import sys -try: - from gi.repository import GLib as gobject -except ImportError: - import gobject +from gi.repository import GLib as GLib logger = logging.getLogger(__name__) @@ -463,7 +460,7 @@ class TaskRunner(LogStub): class SyncRunner(TaskRunner, ITaskListener): """ - I run the task synchronously in a gobject MainLoop. + I run the task synchronously in a GObject MainLoop. """ def __init__(self, verbose=True): @@ -478,11 +475,11 @@ class SyncRunner(TaskRunner, ITaskListener): self._verboseRun = verbose self._skip = skip - self._loop = gobject.MainLoop() + self._loop = GLib.MainLoop() self._task.addListener(self) # only start the task after going into the mainloop, # otherwise the task might complete before we are in it - gobject.timeout_add(0L, self._startWrap, self._task) + GLib.timeout_add(0L, self._startWrap, self._task) self.debug('run loop') self._loop.run() @@ -526,7 +523,7 @@ class SyncRunner(TaskRunner, ITaskListener): self.debug('schedule: scheduling %r(*args=%r, **kwargs=%r)', callable, args, kwargs) - gobject.timeout_add(int(delta * 1000L), c) + GLib.timeout_add(int(delta * 1000L), c) # ITaskListener methods def progressed(self, task, value): diff --git a/whipper/image/table.py b/whipper/image/table.py index 64050ab..2cd6ba7 100644 --- a/whipper/image/table.py +++ b/whipper/image/table.py @@ -339,13 +339,9 @@ class Table(object): values = self._getMusicBrainzValues() # MusicBrainz disc id does not take into account data tracks - # P2.3 - try: - import hashlib - sha1 = hashlib.sha1 - except ImportError: - from sha import sha as sha1 import base64 + import hashlib + sha1 = hashlib.sha1 sha = sha1() diff --git a/whipper/test/test_common_program.py b/whipper/test/test_common_program.py index ef7ca0b..4fba987 100644 --- a/whipper/test/test_common_program.py +++ b/whipper/test/test_common_program.py @@ -15,9 +15,8 @@ class PathTestCase(unittest.TestCase): path = prog.getPath(u'/tmp', DEFAULT_DISC_TEMPLATE, 'mbdiscid', None) - self.assertEqual(path, - unicode('/tmp/unknown/Unknown Artist - mbdiscid/' - 'Unknown Artist - mbdiscid')) + self.assertEqual(path, (u'/tmp/unknown/Unknown Artist - mbdiscid/' + u'Unknown Artist - mbdiscid')) def testStandardTemplateFilled(self): prog = program.Program(config.Config()) @@ -27,9 +26,8 @@ class PathTestCase(unittest.TestCase): path = prog.getPath(u'/tmp', DEFAULT_DISC_TEMPLATE, 'mbdiscid', md, 0) - self.assertEqual(path, - unicode('/tmp/unknown/Jeff Buckley - Grace/' - 'Jeff Buckley - Grace')) + self.assertEqual(path, (u'/tmp/unknown/Jeff Buckley - Grace/' + u'Jeff Buckley - Grace')) def testIssue66TemplateFilled(self): prog = program.Program(config.Config()) From 5311727572abb05fb3bf3326fb0a4ce332593c29 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Wed, 16 Jan 2019 21:21:21 +0000 Subject: [PATCH 04/13] Avoid shadowing built-ins/variables --- whipper/common/program.py | 4 ++-- whipper/common/task.py | 4 ++-- whipper/extern/asyncsub.py | 12 ++++++------ whipper/extern/task/task.py | 16 ++++++++-------- whipper/image/cue.py | 4 ++-- whipper/program/cdparanoia.py | 4 ++-- whipper/program/soxi.py | 8 ++++---- whipper/test/test_program_cdparanoia.py | 4 ++-- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/whipper/common/program.py b/whipper/common/program.py index 383ffcc..f1f932d 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -572,10 +572,10 @@ class Program: return cuePath - def writeLog(self, discName, logger): + def writeLog(self, discName, txt_logger): logPath = common.truncate_filename(discName + '.log') handle = open(logPath, 'w') - log = logger.log(self.result) + log = txt_logger.log(self.result) handle.write(log.encode('utf-8')) handle.close() diff --git a/whipper/common/task.py b/whipper/common/task.py index 5e61933..d58fcea 100644 --- a/whipper/common/task.py +++ b/whipper/common/task.py @@ -115,13 +115,13 @@ class PopenTask(task.Task): os.kill(self._popen.pid, signal.SIGTERM) # self.stop() - def readbytesout(self, bytes): + def readbytesout(self, bytes_stdout): """ Called when bytes have been read from stdout. """ pass - def readbyteserr(self, bytes): + def readbyteserr(self, bytes_stderr): """ Called when bytes have been read from stderr. """ diff --git a/whipper/extern/asyncsub.py b/whipper/extern/asyncsub.py index 76b1e0d..4e4f9a8 100644 --- a/whipper/extern/asyncsub.py +++ b/whipper/extern/asyncsub.py @@ -28,8 +28,8 @@ class Popen(subprocess.Popen): def recv_err(self, maxsize=None): return self._recv('stderr', maxsize) - def send_recv(self, input='', maxsize=None): - return self.send(input), self.recv(maxsize), self.recv_err(maxsize) + def send_recv(self, in_put='', maxsize=None): + return self.send(in_put), self.recv(maxsize), self.recv_err(maxsize) def get_conn_maxsize(self, which, maxsize): if maxsize is None: @@ -44,13 +44,13 @@ class Popen(subprocess.Popen): if subprocess.mswindows: - def send(self, input): + def send(self, in_put): if not self.stdin: return None try: x = msvcrt.get_osfhandle(self.stdin.fileno()) - (errCode, written) = WriteFile(x, input) + (errCode, written) = WriteFile(x, in_put) except ValueError: return self._close('stdin') except (subprocess.pywintypes.error, Exception) as why: @@ -85,7 +85,7 @@ class Popen(subprocess.Popen): else: - def send(self, input): + def send(self, in_put): if not self.stdin: return None @@ -93,7 +93,7 @@ class Popen(subprocess.Popen): return 0 try: - written = os.write(self.stdin.fileno(), input) + written = os.write(self.stdin.fileno(), in_put) except OSError as why: if why.args[0] == errno.EPIPE: # broken pipe return self._close('stdin') diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index c3f86f7..9c51771 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -210,13 +210,13 @@ class Task(LogStub): self.debug('set exception, %r, %r' % ( exception, self.exceptionMessage)) - def schedule(self, delta, callable, *args, **kwargs): + def schedule(self, delta, callable_task, *args, **kwargs): if not self.runner: print("ERROR: scheduling on a task that's altready stopped") import traceback traceback.print_stack() return - self.runner.schedule(self, delta, callable, *args, **kwargs) + self.runner.schedule(self, delta, callable_task, *args, **kwargs) def addListener(self, listener): """ @@ -446,7 +446,7 @@ class TaskRunner(LogStub): raise NotImplementedError # methods for tasks to call - def schedule(self, delta, callable, *args, **kwargs): + def schedule(self, delta, callable_task, *args, **kwargs): """ Schedule a single future call. @@ -507,21 +507,21 @@ class SyncRunner(TaskRunner, ITaskListener): self.debug('exception during start: %r', task.exceptionMessage) self.stopped(task) - def schedule(self, task, delta, callable, *args, **kwargs): + def schedule(self, task, delta, callable_task, *args, **kwargs): def c(): try: self.debug('schedule: calling %r(*args=%r, **kwargs=%r)', - callable, args, kwargs) - callable(*args, **kwargs) + callable_task, args, kwargs) + callable_task(*args, **kwargs) return False except Exception as e: self.debug('exception when calling scheduled callable %r', - callable) + callable_task) task.setException(e) self.stopped(task) raise self.debug('schedule: scheduling %r(*args=%r, **kwargs=%r)', - callable, args, kwargs) + callable_task, args, kwargs) GLib.timeout_add(int(delta * 1000L), c) diff --git a/whipper/image/cue.py b/whipper/image/cue.py index 4088c14..b7aa268 100644 --- a/whipper/image/cue.py +++ b/whipper/image/cue.py @@ -192,14 +192,14 @@ class File: I represent a FILE line in a cue file. """ - def __init__(self, path, format): + def __init__(self, path, file_format): """ @type path: unicode """ assert isinstance(path, unicode), "%r is not unicode" % path self.path = path - self.format = format + self.format = file_format def __repr__(self): return '' % (self.path, self.format) diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index 6fe475a..e533290 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -590,8 +590,8 @@ class AnalyzeTask(ctask.PopenTask): def commandMissing(self): raise common.MissingDependencyException('cd-paranoia') - def readbyteserr(self, bytes): - self._output.append(bytes) + def readbyteserr(self, bytes_stderr): + self._output.append(bytes_stderr) def done(self): if self.cwd: diff --git a/whipper/program/soxi.py b/whipper/program/soxi.py index 72f0caa..07b8f9a 100644 --- a/whipper/program/soxi.py +++ b/whipper/program/soxi.py @@ -35,11 +35,11 @@ class AudioLengthTask(ctask.PopenTask): def commandMissing(self): raise common.MissingDependencyException('soxi') - def readbytesout(self, bytes): - self._output.append(bytes) + def readbytesout(self, bytes_stdout): + self._output.append(bytes_stdout) - def readbyteserr(self, bytes): - self._error.append(bytes) + def readbyteserr(self, bytes_stderr): + self._error.append(bytes_stderr) def failed(self): self.setException(Exception("soxi failed: %s" % "".join(self._error))) diff --git a/whipper/test/test_program_cdparanoia.py b/whipper/test/test_program_cdparanoia.py index 4025469..108b9e4 100644 --- a/whipper/test/test_program_cdparanoia.py +++ b/whipper/test/test_program_cdparanoia.py @@ -75,8 +75,8 @@ class AnalyzeFileTask(cdparanoia.AnalyzeTask): def __init__(self, path): self.command = ['cat', path] - def readbytesout(self, bytes): - self.readbyteserr(bytes) + def readbytesout(self, bytes_stdout): + self.readbyteserr(bytes_stdout) class CacheTestCase(common.TestCase): From af90c1c3381d9654779b80f25dd178f8785d5d02 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 08:42:24 +0000 Subject: [PATCH 05/13] Fix unresolved reference --- whipper/test/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/whipper/test/common.py b/whipper/test/common.py index 49cf109..2df9f23 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -53,7 +53,7 @@ class TestCase(unittest.TestCase): return inst except exception as e: raise Exception('%s raised instead of %s:\n %s' % - (sys.exec_info()[0], exception.__name__, str(e)) + (sys.exc_info()[0], exception.__name__, str(e)) ) else: raise Exception('%s not raised (%r returned)' % From 16b0d8dc295d8576f863c39cffc9009d20a63937 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 09:34:30 +0000 Subject: [PATCH 06/13] Make methods static --- whipper/command/offset.py | 3 ++- whipper/common/program.py | 15 +++++++++++---- whipper/extern/task/task.py | 9 ++++++--- whipper/image/table.py | 6 ++++-- whipper/result/result.py | 3 ++- whipper/test/common.py | 3 ++- whipper/test/test_image_cue.py | 3 ++- whipper/test/test_image_toc.py | 3 ++- 8 files changed, 31 insertions(+), 14 deletions(-) diff --git a/whipper/command/offset.py b/whipper/command/offset.py index 300d90f..444c684 100644 --- a/whipper/command/offset.py +++ b/whipper/command/offset.py @@ -200,7 +200,8 @@ CD in the AccurateRip database.""" os.unlink(path) return "%08x" % v1, "%08x" % v2 - def _foundOffset(self, device, offset): + @staticmethod + def _foundOffset(device, offset): print('\nRead offset of device is: %d.' % offset) info = drive.getDeviceInfo(device) diff --git a/whipper/common/program.py b/whipper/common/program.py index f1f932d..1cee728 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -81,7 +81,8 @@ class Program: self._filter = path.PathFilter(**d) - def setWorkingDirectory(self, workingDirectory): + @staticmethod + def setWorkingDirectory(workingDirectory): if workingDirectory: logger.info('changing to working directory %s', workingDirectory) os.chdir(workingDirectory) @@ -140,7 +141,11 @@ class Program: return self.result - def addDisambiguation(self, template_part, metadata): + def saveRipResult(self): + self._presult.persist() + + @staticmethod + def addDisambiguation(template_part, metadata): """Add disambiguation to template path part string.""" if metadata.catalogNumber: template_part += ' (%s)' % metadata.catalogNumber @@ -218,7 +223,8 @@ class Program: template = re.sub(r'%(\w)', r'%(\1)s', template) return os.path.join(outdir, template % v) - def getCDDB(self, cddbdiscid): + @staticmethod + def getCDDB(cddbdiscid): """ @param cddbdiscid: list of id, tracks, offsets, seconds @@ -440,7 +446,8 @@ class Program: stop = track.getIndex(1).absolute - 1 return start, stop - def verifyTrack(self, runner, trackResult): + @staticmethod + def verifyTrack(runner, trackResult): is_wave = not trackResult.filename.endswith('.flac') t = checksum.CRC32Task(trackResult.filename, is_wave=is_wave) diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index 9c51771..3761f16 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -74,13 +74,16 @@ class LogStub(object): I am a stub for a log interface. """ - def log(self, message, *args): + @staticmethod + def log(message, *args): logger.info(message, *args) - def debug(self, message, *args): + @staticmethod + def debug(message, *args): logger.debug(message, *args) - def warning(self, message, *args): + @staticmethod + def warning(message, *args): logger.warning(message, *args) diff --git a/whipper/image/table.py b/whipper/image/table.py index 2cd6ba7..0b32d4d 100644 --- a/whipper/image/table.py +++ b/whipper/image/table.py @@ -249,7 +249,8 @@ class Table(object): """ return len([t for t in self.tracks if not t.audio]) > 0 - def _cddbSum(self, i): + @staticmethod + def _cddbSum(i): ret = 0 while i > 0: ret += (i % 10) @@ -728,7 +729,8 @@ class Table(object): self.leadout += other.leadout + gap # FIXME logger.debug('fixing leadout, now %d', self.leadout) - def _getSessionGap(self, session): + @staticmethod + def _getSessionGap(session): # From cdrecord multi-session info: # For the first additional session this is 11250 sectors # lead-out/lead-in overhead + 150 sectors for the pre-gap of the first diff --git a/whipper/result/result.py b/whipper/result/result.py index fbde6b2..265514c 100644 --- a/whipper/result/result.py +++ b/whipper/result/result.py @@ -140,7 +140,8 @@ class Logger(object): class EntryPoint(object): name = 'whipper' - def load(self): + @staticmethod + def load(): from whipper.result import logger return logger.WhipperLogger diff --git a/whipper/test/common.py b/whipper/test/common.py index 2df9f23..b42ffc7 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -62,7 +62,8 @@ class TestCase(unittest.TestCase): assertRaises = failUnlessRaises - def readCue(self, name): + @staticmethod + def readCue(name): """ Read a .cue file, and replace the version comment with the current version so we can use it in comparisons. diff --git a/whipper/test/test_image_cue.py b/whipper/test/test_image_cue.py index 3bdbc66..d73333f 100644 --- a/whipper/test/test_image_cue.py +++ b/whipper/test/test_image_cue.py @@ -59,7 +59,8 @@ class KanyeMixedTestCase(unittest.TestCase): class WriteCueFileTestCase(unittest.TestCase): - def testWrite(self): + @staticmethod + def testWrite(): fd, path = tempfile.mkstemp(suffix=u'.whipper.test.cue') os.close(fd) diff --git a/whipper/test/test_image_toc.py b/whipper/test/test_image_toc.py index e7c5596..a51b256 100644 --- a/whipper/test/test_image_toc.py +++ b/whipper/test/test_image_toc.py @@ -353,7 +353,8 @@ class StrokesTestCase(common.TestCase): 'strokes-someday.eac.cue')).read()).decode('utf-8') common.diffStrings(ref, cue) - def _filterCue(self, output): + @staticmethod + def _filterCue(output): # helper to be able to compare our generated .cue with the # EAC-extracted one discard = ['TITLE', 'PERFORMER', 'FLAGS', 'REM'] From e7bfc34c0eb770caf0ef8ff4efc557dd22039195 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 09:57:12 +0000 Subject: [PATCH 07/13] Rename 'throwaway' variables to single underscore Also removed unused ones --- whipper/command/main.py | 2 +- whipper/command/offset.py | 1 - whipper/common/accurip.py | 2 +- whipper/common/checksum.py | 2 +- whipper/common/config.py | 1 - whipper/common/drive.py | 2 +- whipper/common/program.py | 2 -- whipper/extern/freedb.py | 2 +- whipper/image/image.py | 2 +- whipper/image/toc.py | 8 ++++---- whipper/program/arc.py | 2 +- whipper/program/cdparanoia.py | 2 +- whipper/program/cdrdao.py | 2 +- whipper/program/sox.py | 2 +- 14 files changed, 14 insertions(+), 18 deletions(-) diff --git a/whipper/command/main.py b/whipper/command/main.py index 651f748..81f171a 100644 --- a/whipper/command/main.py +++ b/whipper/command/main.py @@ -52,7 +52,7 @@ def main(): return 1 except KeyboardInterrupt: return 2 - except ImportError as e: + except ImportError: raise except task.TaskException as e: if isinstance(e.exception, ImportError): diff --git a/whipper/command/offset.py b/whipper/command/offset.py index 444c684..5745b8f 100644 --- a/whipper/command/offset.py +++ b/whipper/command/offset.py @@ -88,7 +88,6 @@ CD in the AccurateRip database.""" table = t.table logger.debug("CDDB disc id: %r", table.getCDDBDiscId()) - responses = None try: responses = accurip.get_db_entry(table.accuraterip_path()) except accurip.EntryNotFound: diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py index 9085bd5..40199a6 100644 --- a/whipper/common/accurip.py +++ b/whipper/common/accurip.py @@ -236,7 +236,7 @@ def print_report(result): """ Print AccurateRip verification results. """ - for i, track in enumerate(result.tracks): + for _, track in enumerate(result.tracks): status = 'rip NOT accurate' conf = '(not found)' db = 'notfound' diff --git a/whipper/common/checksum.py b/whipper/common/checksum.py index 68ac12b..0891c29 100644 --- a/whipper/common/checksum.py +++ b/whipper/common/checksum.py @@ -47,7 +47,7 @@ class CRC32Task(etask.Task): def _crc32(self): if not self.is_wave: - fd, tmpf = tempfile.mkstemp() + _, tmpf = tempfile.mkstemp() try: subprocess.check_call(['flac', '-d', self.path, '-fo', tmpf]) diff --git a/whipper/common/config.py b/whipper/common/config.py index 8d10935..9ca386a 100644 --- a/whipper/common/config.py +++ b/whipper/common/config.py @@ -156,7 +156,6 @@ class Config: section = 'drive:' + urllib.quote('%s:%s:%s' % ( vendor, model, release)) self._parser.add_section(section) - __pychecker__ = 'no-local' for key in ['vendor', 'model', 'release']: self._parser.set(section, key, locals()[key].strip()) diff --git a/whipper/common/drive.py b/whipper/common/drive.py index 226e04d..21d02ff 100644 --- a/whipper/common/drive.py +++ b/whipper/common/drive.py @@ -66,6 +66,6 @@ def getDeviceInfo(path): except ImportError: return None device = cdio.Device(path) - ok, vendor, model, release = device.get_hwinfo() + _, vendor, model, release = device.get_hwinfo() return vendor, model, release diff --git a/whipper/common/program.py b/whipper/common/program.py index 1cee728..79ad105 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -259,10 +259,8 @@ class Program: ittoc.getAudioTracks())) logger.debug('MusicBrainz submit url: %r', ittoc.getMusicBrainzSubmitURL()) - ret = None metadatas = None - e = None for _ in range(0, 4): try: diff --git a/whipper/extern/freedb.py b/whipper/extern/freedb.py index d08fcb7..1e71901 100644 --- a/whipper/extern/freedb.py +++ b/whipper/extern/freedb.py @@ -134,7 +134,7 @@ def perform_lookup(disc_id, freedb_server, freedb_port): if len(matches) > 0: # for each result, query FreeDB for XMCD file data - for (category, disc_id, title) in matches: + for (category, disc_id, _) in matches: sleep(1) # add a slight delay to keep the server happy query = freedb_command(freedb_server, diff --git a/whipper/image/image.py b/whipper/image/image.py index 0d05893..a499285 100644 --- a/whipper/image/image.py +++ b/whipper/image/image.py @@ -192,7 +192,7 @@ class ImageEncodeTask(task.MultiSeparateTask): path = image.getRealPath(index.path) assert isinstance(path, unicode), "%r is not unicode" % path logger.debug('schedule encode of %r', path) - root, ext = os.path.splitext(os.path.basename(path)) + root, _ = os.path.splitext(os.path.basename(path)) outpath = os.path.join(outdir, root + '.' + 'flac') logger.debug('schedule encode to %r', outpath) taskk = encode.FlacEncodeTask( diff --git a/whipper/image/toc.py b/whipper/image/toc.py index be5e521..3d03d13 100644 --- a/whipper/image/toc.py +++ b/whipper/image/toc.py @@ -117,7 +117,7 @@ class Sources: """ Retrieve the source used at the given offset. """ - for i, (c, o, s) in enumerate(self._sources): + for i, (_, o, _) in enumerate(self._sources): if offset < o: return self._sources[i - 1] @@ -127,7 +127,7 @@ class Sources: """ Retrieve the absolute offset of the first source for this counter """ - for i, (c, o, s) in enumerate(self._sources): + for i, (c, _, _) in enumerate(self._sources): if c == counter: return self._sources[i][1] @@ -151,7 +151,7 @@ class TocFile(object): def _index(self, currentTrack, i, absoluteOffset, trackOffset): absolute = absoluteOffset + trackOffset # this may be in a new source, so calculate relative - c, o, s = self._sources.get(absolute) + c, _, s = self._sources.get(absolute) logger.debug('at abs offset %d, we are in source %r', absolute, s) counterStart = self._sources.getCounterStart(c) @@ -341,7 +341,7 @@ class TocFile(object): continue length = common.msfToFrames(m.group('length')) - c, o, s = self._sources.get(absoluteOffset) + c, _, s = self._sources.get(absoluteOffset) logger.debug('at abs offset %d, we are in source %r', absoluteOffset, s) counterStart = self._sources.getCounterStart(c) diff --git a/whipper/program/arc.py b/whipper/program/arc.py index e112d31..46b8fdf 100644 --- a/whipper/program/arc.py +++ b/whipper/program/arc.py @@ -31,7 +31,7 @@ def accuraterip_checksum(f, track_number, total_tracks, wave=False, v2=False): if not wave: flac.stdout.close() - out, err = arc.communicate() + out, _ = arc.communicate() if not wave: flac.wait() diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index e533290..254e099 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -263,7 +263,7 @@ class ReadTrackTask(task.Task): stopTrack = 0 stopOffset = self._stop - for i, t in enumerate(self._table.tracks): + for i, _ in enumerate(self._table.tracks): if self._table.getTrackStart(i + 1) <= self._start: startTrack = i + 1 startOffset = self._start - self._table.getTrackStart(i + 1) diff --git a/whipper/program/cdrdao.py b/whipper/program/cdrdao.py index 36a0f89..d76882a 100644 --- a/whipper/program/cdrdao.py +++ b/whipper/program/cdrdao.py @@ -179,7 +179,7 @@ def version(): Return cdrdao version as a string. """ cdrdao = Popen(CDRDAO, stderr=PIPE) - out, err = cdrdao.communicate() + _, err = cdrdao.communicate() if cdrdao.returncode != 1: logger.warning("cdrdao version detection failed: " "return code is %s", cdrdao.returncode) diff --git a/whipper/program/sox.py b/whipper/program/sox.py index 1ec54d3..51955d9 100644 --- a/whipper/program/sox.py +++ b/whipper/program/sox.py @@ -18,7 +18,7 @@ def peak_level(track_path): logger.warning("SoX peak detection failed: file not found") return None sox = Popen([SOX, track_path, "-n", "stats", "-b", "16"], stderr=PIPE) - out, err = sox.communicate() + _, err = sox.communicate() if sox.returncode: logger.warning("SoX peak detection failed: %s", sox.returncode) return None From 6bb585e1a547d0b41e0e596c3eda2ee88a089677 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 10:42:48 +0000 Subject: [PATCH 08/13] Replace if ... else ... with simpler statement --- whipper/program/cdparanoia.py | 5 +---- whipper/program/cdrdao.py | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index 254e099..465ffd1 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -598,10 +598,7 @@ class AnalyzeTask(ctask.PopenTask): shutil.rmtree(self.cwd) output = "".join(self._output) m = _OK_RE.search(output) - if m: - self.defeatsCache = True - else: - self.defeatsCache = False + self.defeatsCache = bool(m) def failed(self): # cdparanoia exits with return code 1 if it can't determine diff --git a/whipper/program/cdrdao.py b/whipper/program/cdrdao.py index d76882a..8bfad7f 100644 --- a/whipper/program/cdrdao.py +++ b/whipper/program/cdrdao.py @@ -168,10 +168,7 @@ def DetectCdr(device): cmd = [CDRDAO, 'disk-info', '-v1', '--device', device] logger.debug("executing %r", cmd) p = Popen(cmd, stdout=PIPE, stderr=PIPE) - if 'CD-R medium : n/a' in p.stdout.read(): - return False - else: - return True + return 'CD-R medium : n/a' not in p.stdout.read() def version(): From ad66af900f2ed5466e1802612d8353e02ee61a12 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 11:21:33 +0000 Subject: [PATCH 09/13] Remove erroneous extra format argument --- whipper/command/offset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/whipper/command/offset.py b/whipper/command/offset.py index 5745b8f..8dace36 100644 --- a/whipper/command/offset.py +++ b/whipper/command/offset.py @@ -132,7 +132,7 @@ CD in the AccurateRip database.""" logger.warning('cannot rip with offset %d...', offset) continue - logger.debug('AR checksums calculated: %s %s', archecksums) + logger.debug('AR checksums calculated: %s', archecksums) c, i = match(archecksums, 1, responses) if c: From fe3624173038290ba6c5c682cc8f0923b35bd04d Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 11:24:08 +0000 Subject: [PATCH 10/13] Don't assign the result of a function that returns no value --- whipper/common/encode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/whipper/common/encode.py b/whipper/common/encode.py index 23af808..237daa9 100644 --- a/whipper/common/encode.py +++ b/whipper/common/encode.py @@ -60,7 +60,7 @@ class FlacEncodeTask(task.Task): self.schedule(0.0, self._flac_encode) def _flac_encode(self): - self.new_path = flac.encode(self.track_path, self.track_out_path) + flac.encode(self.track_path, self.track_out_path) self.stop() From cf923cc9ccbcdabac783a1222357ff56f8dcf1d9 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 11:25:16 +0000 Subject: [PATCH 11/13] Review existing comments and add new ones Also clarified a statement --- whipper/command/cd.py | 5 ++++- whipper/command/main.py | 1 + whipper/common/cache.py | 4 ++-- whipper/common/program.py | 1 + whipper/common/task.py | 1 + whipper/extern/freedb.py | 1 + whipper/extern/task/task.py | 3 +++ whipper/image/image.py | 2 ++ whipper/program/cdparanoia.py | 2 ++ whipper/result/logger.py | 2 -- whipper/test/common.py | 1 + 11 files changed, 18 insertions(+), 5 deletions(-) diff --git a/whipper/command/cd.py b/whipper/command/cd.py index aed96d8..38aad2e 100644 --- a/whipper/command/cd.py +++ b/whipper/command/cd.py @@ -66,6 +66,7 @@ disc and track template are: class _CD(BaseCommand): eject = True + # XXX: Pylint, parameters differ from overridden 'add_arguments' method @staticmethod def add_arguments(parser): parser.add_argument('-R', '--release-id', @@ -205,6 +206,7 @@ class Info(_CD): # Requires opts.device + # XXX: Pylint, parameters differ from overridden 'add_arguments' method def add_arguments(self): _CD.add_arguments(self.parser) @@ -228,6 +230,7 @@ Log files will log the path to tracks relative to this directory. # Requires opts.record # Requires opts.device + # XXX: Pylint, parameters differ from overridden 'add_arguments' method def add_arguments(self): loggers = list(result.getLoggers()) default_offset = None @@ -246,7 +249,6 @@ Log files will log the path to tracks relative to this directory. default='whipper', help=("logger to use (choose from: '%s" % "', '".join(loggers) + "')")) - # FIXME: get from config self.parser.add_argument('-o', '--offset', action="store", dest="offset", default=default_offset, @@ -415,6 +417,7 @@ Log files will log the path to tracks relative to this directory. len(self.itable.tracks), extra)) break + # FIXME: catching too general exception (Exception) except Exception as e: logger.debug('got exception %r on try %d', e, tries) diff --git a/whipper/command/main.py b/whipper/command/main.py index 81f171a..e6ced3c 100644 --- a/whipper/command/main.py +++ b/whipper/command/main.py @@ -45,6 +45,7 @@ def main(): logger.critical("SystemError: %s", e) if (isinstance(e, common.EjectError) and cmd.options.eject in ('failure', 'always')): + # XXX: Pylint, instance of 'SystemError' has no 'device' member eject_device(e.device) return 255 except RuntimeError as e: diff --git a/whipper/common/cache.py b/whipper/common/cache.py index 18fe6b3..cf311a6 100644 --- a/whipper/common/cache.py +++ b/whipper/common/cache.py @@ -104,8 +104,8 @@ class Persister: try: self.object = pickle.load(handle) logger.debug('loaded persisted object from %r', self._path) + # FIXME: catching too general exception (Exception) except Exception as e: - # TODO: restrict kind of caught exceptions? # can fail for various reasons; in that case, pretend we didn't # load it logger.debug(e) @@ -127,7 +127,7 @@ class PersistedCache: try: os.makedirs(self.path) except OSError as e: - if e.errno != 17: # FIXME + if e.errno != os.errno.EEXIST: # FIXME: errno 17 is 'File Exists' raise def _getPath(self, key): diff --git a/whipper/common/program.py b/whipper/common/program.py index 79ad105..6906ef1 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -297,6 +297,7 @@ class Program: print('Type : %s' % metadata.releaseType) if metadata.barcode: print("Barcode : %s" % metadata.barcode) + # TODO: Add test for non ASCII catalog numbers: see issue #215 if metadata.catalogNumber: print("Cat no : %s" % metadata.catalogNumber.encode('utf-8')) diff --git a/whipper/common/task.py b/whipper/common/task.py index d58fcea..1c3501f 100644 --- a/whipper/common/task.py +++ b/whipper/common/task.py @@ -87,6 +87,7 @@ class PopenTask(task.Task): return self._done() + # FIXME: catching too general exception (Exception) except Exception as e: logger.debug('exception during _read(): %s', e) self.setException(e) diff --git a/whipper/extern/freedb.py b/whipper/extern/freedb.py index 1e71901..6359a91 100644 --- a/whipper/extern/freedb.py +++ b/whipper/extern/freedb.py @@ -134,6 +134,7 @@ def perform_lookup(disc_id, freedb_server, freedb_port): if len(matches) > 0: # for each result, query FreeDB for XMCD file data + # XXX: Pylint, redefining argument with the local name 'disc_id' for (category, disc_id, _) in matches: sleep(1) # add a slight delay to keep the server happy diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index 3761f16..77c0bcc 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -238,6 +238,7 @@ class Task(LogStub): method = getattr(l, methodName) try: method(self, *args, **kwargs) + # FIXME: catching too general exception (Exception) except Exception as e: self.setException(e) @@ -350,6 +351,7 @@ class BaseMultiTask(Task, ITaskListener): task.start(self.runner) self.debug('BaseMultiTask.next(): started task %d of %d: %r', self._task, len(self.tasks), task) + # FIXME: catching too general exception (Exception) except Exception as e: self.setException(e) self.debug('Got exception during next: %r', self.exceptionMessage) @@ -503,6 +505,7 @@ class SyncRunner(TaskRunner, ITaskListener): try: self.debug('start task %r' % task) task.start(self) + # FIXME: catching too general exception (Exception) except Exception as e: # getExceptionMessage uses global exception state that doesn't # hang around, so store the message diff --git a/whipper/image/image.py b/whipper/image/image.py index a499285..e57ad3c 100644 --- a/whipper/image/image.py +++ b/whipper/image/image.py @@ -121,6 +121,7 @@ class ImageVerifyTask(task.MultiSeparateTask): task.MultiSeparateTask.__init__(self) self._image = image + # XXX: Pylint, redefining name 'cue' from outer scope (import) cue = image.cue self._tasks = [] self.lengths = {} @@ -183,6 +184,7 @@ class ImageEncodeTask(task.MultiSeparateTask): task.MultiSeparateTask.__init__(self) self._image = image + # XXX: Pylint, redefining name 'cue' from outer scope (import) cue = image.cue self._tasks = [] self.lengths = {} diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index 465ffd1..29d2176 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -538,6 +538,7 @@ class ReadVerifyTrackTask(task.MultiSeparateTask): try: logger.debug('moving to final path %r', self.path) os.rename(self._tmppath, self.path) + # FIXME: catching too general exception (Exception) except Exception as e: logger.debug('exception while moving to final ' 'path %r: %s', self.path, e) @@ -546,6 +547,7 @@ class ReadVerifyTrackTask(task.MultiSeparateTask): os.unlink(self._tmppath) else: logger.debug('stop: exception %r', self.exception) + # FIXME: catching too general exception (Exception) except Exception as e: print('WARNING: unhandled exception %r' % (e, )) diff --git a/whipper/result/logger.py b/whipper/result/logger.py index eda4af0..6da9f52 100644 --- a/whipper/result/logger.py +++ b/whipper/result/logger.py @@ -98,8 +98,6 @@ class WhipperLogger(result.Logger): # For every track include information in the TOC for t in table.tracks: - # FIXME: what happens to a track start over 60 minutes ? - # Answer: tested empirically, everything seems OK start = t.getIndex(1).absolute length = table.getTrackLength(t.number) end = table.getTrackEnd(t.number) diff --git a/whipper/test/common.py b/whipper/test/common.py index b42ffc7..a35b96e 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -46,6 +46,7 @@ class TestCase(unittest.TestCase): # and we'd like to check for the actual exception under TaskException, # so override the way twisted.trial.unittest does, without failure + # XXX: Pylint, method could be a function (no-self-use) def failUnlessRaises(self, exception, f, *args, **kwargs): try: result = f(*args, **kwargs) From f79fb32340de450a519e848ec50d35b6c5028687 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 14:13:04 +0000 Subject: [PATCH 12/13] Remove broken assertion --- whipper/command/cd.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/whipper/command/cd.py b/whipper/command/cd.py index 38aad2e..f3ce661 100644 --- a/whipper/command/cd.py +++ b/whipper/command/cd.py @@ -157,10 +157,6 @@ class _CD(BaseCommand): "full table's mb id %s differs from toc id mb %s" % ( self.itable.getMusicBrainzDiscId(), self.ittoc.getMusicBrainzDiscId()) - assert self.itable.accuraterip_path() == \ - self.ittoc.accuraterip_path(), \ - "full table's AR URL %s differs from toc AR URL %s" % ( - self.itable.accuraterip_url(), self.ittoc.accuraterip_url()) if self.program.metadata: self.program.metadata.discid = self.ittoc.getMusicBrainzDiscId() From 9dc5702e193948f1772174089572121675c6005f Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Thu, 17 Jan 2019 14:17:38 +0000 Subject: [PATCH 13/13] Fix string concatenation bug --- whipper/extern/task/task.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index 77c0bcc..b7e8d6f 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -191,8 +191,8 @@ class Task(LogStub): # for now if str(exception): msg = ": %s" % str(exception) - line = "exception %(exc)s at %(filename)s:%(line)s: " - "%(func)s()%(msg)s" % locals() + line = ("exception %(exc)s at %(filename)s:%(line)s: " + "%(func)s()%(msg)s" % locals()) self.exception = exception self.exceptionMessage = line