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/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..f3ce661 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', @@ -156,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() @@ -191,6 +188,8 @@ class _CD(BaseCommand): if self.options.eject in ('success', 'always'): utils.eject_device(self.device) + return None + def doCommand(self): pass @@ -203,6 +202,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) @@ -226,6 +226,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 @@ -244,7 +245,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, @@ -413,6 +413,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 adaec7b..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: @@ -52,7 +53,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): @@ -74,11 +75,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..8dace36 100644 --- a/whipper/command/offset.py +++ b/whipper/command/offset.py @@ -88,13 +88,12 @@ 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: 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)) @@ -133,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: @@ -170,6 +169,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,9 +197,10 @@ CD in the AccurateRip database.""" ) os.unlink(path) - return ("%08x" % v1, "%08x" % v2) + 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/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/cache.py b/whipper/common/cache.py index 5305ed3..cf311a6 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 @@ -104,12 +104,11 @@ 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) - pass def delete(self): self.object = None @@ -128,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/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/common.py b/whipper/common/common.py index 1b46d4e..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): @@ -285,9 +284,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/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 37950fd..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) + return vendor, model, release 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() 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/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 diff --git a/whipper/common/program.py b/whipper/common/program.py index ad468a2..6906ef1 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,8 +141,12 @@ class Program: return self.result - def addDisambiguation(self, template_part, metadata): - "Add disambiguation to template path part string." + 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 elif metadata.barcode: @@ -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 @@ -253,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: @@ -293,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')) @@ -327,7 +332,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 +351,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,9 +443,10 @@ class Program: start = index.absolute stop = track.getIndex(1).absolute - 1 - return (start, stop) + 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) @@ -572,10 +578,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/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/common/task.py b/whipper/common/task.py index 5e61933..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) @@ -115,13 +116,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 3ee0fef..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,16 +44,16 @@ 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), 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 @@ -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,8 +93,8 @@ class Popen(subprocess.Popen): return 0 try: - written = os.write(self.stdin.fileno(), input) - except OSError, why: + written = os.write(self.stdin.fileno(), in_put) + 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/freedb.py b/whipper/extern/freedb.py index 48a3deb..6359a91 100644 --- a/whipper/extern/freedb.py +++ b/whipper/extern/freedb.py @@ -134,7 +134,8 @@ 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: + # 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 query = freedb_command(freedb_server, @@ -145,7 +146,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/extern/task/task.py b/whipper/extern/task/task.py index 250ccd6..b7e8d6f 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__) @@ -77,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) @@ -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 @@ -213,13 +213,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): """ @@ -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) @@ -449,7 +451,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. @@ -463,7 +465,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 +480,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() @@ -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 @@ -510,23 +513,23 @@ 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) - 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/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/image/image.py b/whipper/image/image.py index 93120cc..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 = {} @@ -192,7 +194,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( @@ -205,7 +207,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..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) @@ -339,13 +340,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() @@ -732,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 @@ -824,7 +822,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/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 74871a4..29d2176 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 @@ -264,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) @@ -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') @@ -540,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) @@ -548,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, )) @@ -592,18 +592,15 @@ 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: 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 acaebd9..8bfad7f 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: @@ -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(): @@ -179,7 +176,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 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/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/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/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 ad2c084..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) @@ -53,7 +54,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)' % @@ -62,7 +63,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. @@ -71,7 +73,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 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()) 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'] 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):