From 64dd9d843ab508651863fa30c73ab10a5b1e97f5 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Sat, 10 Aug 2019 11:45:17 +0900 Subject: [PATCH 1/8] Initial pass on python 3 port Given the imminent end-of-life for Python 2, I didn't bother making the codebase compatible with both. Signed-off-by: Drew DeVault --- whipper/command/cd.py | 25 +++-- whipper/command/image.py | 1 - whipper/command/mblookup.py | 4 +- whipper/command/offset.py | 2 +- whipper/common/accurip.py | 6 +- whipper/common/cache.py | 6 +- whipper/common/common.py | 12 +-- whipper/common/config.py | 17 ++-- whipper/common/directory.py | 14 +-- whipper/common/mbngs.py | 8 +- whipper/common/path.py | 10 +- whipper/common/program.py | 45 +++++---- whipper/extern/asyncsub.py | 6 +- whipper/extern/freedb.py | 67 +++++-------- whipper/extern/task/task.py | 5 +- whipper/image/cue.py | 16 +-- whipper/image/image.py | 12 +-- whipper/image/table.py | 42 ++++---- whipper/image/toc.py | 12 +-- whipper/program/arc.py | 2 +- whipper/program/cdparanoia.py | 13 ++- whipper/program/cdrdao.py | 6 +- whipper/program/soxi.py | 8 +- whipper/test/common.py | 2 +- whipper/test/test_command_mblookup.py | 14 +-- whipper/test/test_common_common.py | 4 +- whipper/test/test_common_config.py | 2 +- whipper/test/test_common_mbngs.py | 136 +++++++++++++------------- whipper/test/test_common_path.py | 16 +-- whipper/test/test_common_program.py | 16 +-- whipper/test/test_image_cue.py | 16 +-- whipper/test/test_image_toc.py | 30 +++--- whipper/test/test_program_soxi.py | 16 +-- 33 files changed, 274 insertions(+), 317 deletions(-) diff --git a/whipper/command/cd.py b/whipper/command/cd.py index fbfe3b70..a45fd9e9 100644 --- a/whipper/command/cd.py +++ b/whipper/command/cd.py @@ -37,8 +37,8 @@ SILENT = 0 MAX_TRIES = 5 -DEFAULT_TRACK_TEMPLATE = u'%r/%A - %d/%t. %a - %n' -DEFAULT_DISC_TEMPLATE = u'%r/%A - %d/%A - %d' +DEFAULT_TRACK_TEMPLATE = '%r/%A - %d/%t. %a - %n' +DEFAULT_DISC_TEMPLATE = '%r/%A - %d/%A - %d' TEMPLATE_DESCRIPTION = ''' Tracks are named according to the track template, filling in the variables @@ -137,7 +137,7 @@ def do(self): if getattr(self.options, 'working_directory', False): os.chdir(os.path.expanduser(self.options.working_directory)) if hasattr(self.options, 'output_directory'): - out_bpath = self.options.output_directory.decode('utf-8') + out_bpath = self.options.output_directory # Needed to preserve cdrdao's tocfile out_fpath = self.program.getPath(out_bpath, self.options.disc_template, @@ -295,10 +295,9 @@ def handle_arguments(self): self.options.output_directory = os.path.expanduser( self.options.output_directory) - self.options.track_template = self.options.track_template.decode( - 'utf-8') + self.options.track_template = self.options.track_template validate_template(self.options.track_template, 'track') - self.options.disc_template = self.options.disc_template.decode('utf-8') + self.options.disc_template = self.options.disc_template validate_template(self.options.disc_template, 'disc') if self.options.offset is None: @@ -323,7 +322,7 @@ def handle_arguments(self): def doCommand(self): self.program.setWorkingDirectory(self.options.working_directory) - self.program.outdir = self.options.output_directory.decode('utf-8') + self.program.outdir = self.options.output_directory self.program.result.offset = int(self.options.offset) self.program.result.overread = self.options.overread self.program.result.logger = self.options.logger @@ -336,13 +335,11 @@ def doCommand(self): if os.path.exists(dirname): logs = glob.glob(os.path.join(dirname, '*.log')) if logs: - msg = ("output directory %s is a finished rip" % - dirname.encode('utf-8')) + msg = ("output directory %s is a finished rip" % dirname) logger.debug(msg) raise RuntimeError(msg) else: - logger.info("creating output directory %s", - dirname.encode('utf-8')) + logger.info("creating output directory %s", dirname) os.makedirs(dirname) # FIXME: turn this into a method @@ -366,7 +363,7 @@ def _ripIfNotRipped(number): logger.debug('ripIfNotRipped: path %r', path) trackResult.number = number - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path trackResult.filename = path if number > 0: trackResult.pregap = self.itable.tracks[number - 1].getPregap() @@ -385,7 +382,7 @@ def _ripIfNotRipped(number): logger.info('verifying track %d of %d: %s', number, len(self.itable.tracks), - os.path.basename(path).encode('utf-8')) + os.path.basename(path)) if not self.program.verifyTrack(self.runner, trackResult): logger.warning('verification failed, reripping...') os.unlink(path) @@ -403,7 +400,7 @@ def _ripIfNotRipped(number): extra = " (try %d)" % tries logger.info('ripping track %d of %d%s: %s', number, len(self.itable.tracks), extra, - os.path.basename(path).encode('utf-8')) + os.path.basename(path)) try: logger.debug('ripIfNotRipped: track %d, try %d', number, tries) diff --git a/whipper/command/image.py b/whipper/command/image.py index 89d326e2..a49b2852 100644 --- a/whipper/command/image.py +++ b/whipper/command/image.py @@ -45,7 +45,6 @@ def do(self): runner = task.SyncRunner() for arg in self.options.cuefile: - arg = arg.decode('utf-8') cueImage = image.Image(arg) cueImage.setup(runner) diff --git a/whipper/command/mblookup.py b/whipper/command/mblookup.py index dc4fcb42..9ecbfba6 100644 --- a/whipper/command/mblookup.py +++ b/whipper/command/mblookup.py @@ -17,7 +17,7 @@ def add_arguments(self): def do(self): try: - discId = unicode(self.options.mbdiscid) + discId = str(self.options.mbdiscid) except IndexError: print('Please specify a MusicBrainz disc id.') return 3 @@ -29,7 +29,7 @@ def do(self): print('- Release %d:' % (i + 1, )) print(' Artist: %s' % md.artist.encode('utf-8')) print(' Title: %s' % md.title.encode('utf-8')) - print(' Type: %s' % unicode(md.releaseType).encode('utf-8')) # noqa: E501 + print(' Type: %s' % str(md.releaseType).encode('utf-8')) # noqa: E501 print(' URL: %s' % md.url) print(' Tracks: %d' % len(md.tracks)) if md.catalogNumber: diff --git a/whipper/command/offset.py b/whipper/command/offset.py index 9e1f61f6..94f7b105 100644 --- a/whipper/command/offset.py +++ b/whipper/command/offset.py @@ -177,7 +177,7 @@ def _arcs(self, runner, table, track, offset): logger.debug('ripping track %r with offset %d...', track, offset) fd, path = tempfile.mkstemp( - suffix=u'.track%02d.offset%d.whipper.wav' % ( + suffix='.track%02d.offset%d.whipper.wav' % ( track, offset)) os.close(fd) diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py index 6133ad75..0cd07879 100644 --- a/whipper/common/accurip.py +++ b/whipper/common/accurip.py @@ -60,7 +60,7 @@ def __init__(self, data): position, so track 1 will have array index 0, track 2 will have array index 1, and so forth. HTOA and other hidden tracks are not included. """ - self.num_tracks = struct.unpack("B", data[0])[0] + self.num_tracks = data[0] self.discId1 = "%08x" % struct.unpack(". -import ConfigParser import codecs +import configparser import os.path import shutil import tempfile -import urllib -from urlparse import urlparse +from urllib.parse import urlparse, quote from whipper.common import directory @@ -37,7 +36,7 @@ class Config: def __init__(self, path=None): self._path = path or directory.config_path() - self._parser = ConfigParser.SafeConfigParser() + self._parser = configparser.ConfigParser() self.open() @@ -51,7 +50,7 @@ def open(self): len(self._parser.sections())) def write(self): - fd, path = tempfile.mkstemp(suffix=u'.whipperrc') + fd, path = tempfile.mkstemp(suffix='.whipperrc') handle = os.fdopen(fd, 'w') self._parser.write(handle) handle.close() @@ -64,7 +63,7 @@ def _getter(self, suffix, section, option): method = getattr(self._parser, methodName) try: return method(section, option) - except (ConfigParser.NoSectionError, ConfigParser.NoOptionError): + except (configparser.NoSectionError, configparser.NoOptionError): return None def get(self, section, option): @@ -102,7 +101,7 @@ def getReadOffset(self, vendor, model, release): try: return int(self._parser.get(section, 'read_offset')) - except ConfigParser.NoOptionError: + except configparser.NoOptionError: raise KeyError("Could not find read_offset for %s/%s/%s" % ( vendor, model, release)) @@ -121,7 +120,7 @@ def getDefeatsCache(self, vendor, model, release): try: return self._parser.get(section, 'defeats_cache') == 'True' - except ConfigParser.NoOptionError: + except configparser.NoOptionError: raise KeyError("Could not find defeats_cache for %s/%s/%s" % ( vendor, model, release)) @@ -153,7 +152,7 @@ def _findOrCreateDriveSection(self, vendor, model, release): try: section = self._findDriveSection(vendor, model, release) except KeyError: - section = 'drive:' + urllib.quote('%s:%s:%s' % ( + section = 'drive:' + quote('%s:%s:%s' % ( vendor, model, release)) self._parser.add_section(section) for key in ['vendor', 'model', 'release']: diff --git a/whipper/common/directory.py b/whipper/common/directory.py index f351c029..910a42e9 100644 --- a/whipper/common/directory.py +++ b/whipper/common/directory.py @@ -23,16 +23,16 @@ def config_path(): - path = join(getenv('XDG_CONFIG_HOME') or join(expanduser('~'), u'.config'), - u'whipper') + path = join(getenv('XDG_CONFIG_HOME') or join(expanduser('~'), '.config'), + 'whipper') if not exists(path): makedirs(path) - return join(path, u'whipper.conf') + return join(path, 'whipper.conf') def cache_path(name=None): - path = join(getenv('XDG_CACHE_HOME') or join(expanduser('~'), u'.cache'), - u'whipper') + path = join(getenv('XDG_CACHE_HOME') or join(expanduser('~'), '.cache'), + 'whipper') if name: path = join(path, name) if not exists(path): @@ -42,8 +42,8 @@ def cache_path(name=None): def data_path(name=None): path = join(getenv('XDG_DATA_HOME') or - join(expanduser('~'), u'.local/share'), - u'whipper') + join(expanduser('~'), '.local/share'), + 'whipper') if name: path = join(path, name) if not exists(path): diff --git a/whipper/common/mbngs.py b/whipper/common/mbngs.py index d39dc878..50170738 100644 --- a/whipper/common/mbngs.py +++ b/whipper/common/mbngs.py @@ -21,7 +21,7 @@ """ Handles communication with the MusicBrainz server using NGS. """ -import urllib2 +from urllib.error import HTTPError import whipper @@ -61,7 +61,7 @@ class DiscMetadata(object): :param artist: artist(s) name :param sortName: release artist sort name :param release: earliest release date, in YYYY-MM-DD - :type release: unicode + :type release: str :param title: title of the disc (with disambiguation) :param releaseTitle: title of the release (without disambiguation) :type tracks: list of :any:`TrackMetadata` @@ -152,7 +152,7 @@ def _getWorks(recording): """Get "performance of" works out of a recording.""" works = [] valid_work_rel_types = [ - u'a3005666-a872-32c3-ad06-98af558e99b0', # "Performance" + 'a3005666-a872-32c3-ad06-98af558e99b0', # "Performance" ] if 'work-relation-list' in recording: for work in recording['work-relation-list']: @@ -298,7 +298,7 @@ def musicbrainz(discid, country=None, record=False): result = musicbrainzngs.get_releases_by_discid( discid, includes=["artists", "recordings", "release-groups"]) except musicbrainzngs.ResponseError as e: - if isinstance(e.cause, urllib2.HTTPError): + if isinstance(e.cause, HTTPError): if e.cause.code == 404: raise NotFoundException(e) else: diff --git a/whipper/common/path.py b/whipper/common/path.py index 8d628fbe..7c01d8ff 100644 --- a/whipper/common/path.py +++ b/whipper/common/path.py @@ -50,18 +50,16 @@ def separators(path): # change all fancy single/double quotes to normal quotes if self._quotes: - path = re.sub(ur'[\xc2\xb4\u2018\u2019\u201b]', "'", path, - re.UNICODE) - path = re.sub(ur'[\u201c\u201d\u201f]', '"', path, re.UNICODE) + path = re.sub(r'[\xc2\xb4\u2018\u2019\u201b]', "'", path) + path = re.sub(r'[\u201c\u201d\u201f]', '"', path) if self._special: path = separators(path) - path = re.sub(r'[*?&!\'\"$()`{}\[\]<>]', - '_', path, re.UNICODE) + path = re.sub(r'[*?&!\'\"$()`{}\[\]<>]', '_', path) 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) return path diff --git a/whipper/common/program.py b/whipper/common/program.py index 5cb1e79d..57748624 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -47,7 +47,7 @@ class Program: :vartype metadata: mbngs.DiscMetadata :cvar result: the rip's result :vartype result: result.RipResult - :vartype outdir: unicode + :vartype outdir: str :vartype config: whipper.common.config.Config """ @@ -197,8 +197,8 @@ def getPath(self, outdir, template, mbdiscid, metadata, track_number=None): - %x: audio extension, lowercase - %X: audio extension, uppercase """ - assert isinstance(outdir, unicode), "%r is not unicode" % outdir - assert isinstance(template, unicode), "%r is not unicode" % template + assert isinstance(outdir, str), "%r is not str" % outdir + assert isinstance(template, str), "%r is not str" % template v = {} v['A'] = 'Unknown Artist' v['d'] = mbdiscid # fallback for title @@ -228,7 +228,7 @@ def getPath(self, outdir, template, mbdiscid, metadata, track_number=None): if metadata.releaseType: v['R'] = metadata.releaseType v['r'] = metadata.releaseType.lower() - if track_number > 0: + if track_number is not None and track_number > 0: v['a'] = self._filter.filter( metadata.tracks[track_number - 1].artist) v['s'] = self._filter.filter( @@ -307,8 +307,8 @@ def getMusicBrainz(self, ittoc, mbdiscid, release=None, country=None, print('\nMatching releases:') for metadata in metadatas: - print('\nArtist : %s' % metadata.artist.encode('utf-8')) - print('Title : %s' % metadata.title.encode('utf-8')) + print('\nArtist : %s' % metadata.artist) + print('Title : %s' % metadata.title) print('Duration: %s' % common.formatTime( metadata.duration / 1000.0)) print('URL : %s' % metadata.url) @@ -318,8 +318,7 @@ def getMusicBrainz(self, ittoc, mbdiscid, release=None, country=None, 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')) + print("Cat no : %s" % metadata.catalogNumber) delta = abs(metadata.duration - ittoc.duration()) if delta not in deltas: @@ -346,8 +345,8 @@ def getMusicBrainz(self, ittoc, mbdiscid, release=None, country=None, metadatas) if len(metadatas) == 1: logger.info('picked requested release id %s', release) - print('Artist: %s' % metadatas[0].artist.encode('utf-8')) - print('Title : %s' % metadatas[0].title.encode('utf-8')) + print('Artist: %s' % metadatas[0].artist) + print('Title : %s' % metadatas[0].title) elif not metadatas: logger.warning("requested release id '%s', but none of " "the found releases match", release) @@ -374,8 +373,8 @@ def getMusicBrainz(self, ittoc, mbdiscid, release=None, country=None, logger.warning('picked closest match in duration. ' 'Others may be wrong in MusicBrainz, ' 'please correct') - print('Artist : %s' % artist.encode('utf-8')) - print('Title : %s' % metadatas[0].title.encode('utf-8')) + print('Artist : %s' % artist) + print('Title : %s' % metadatas[0].title) # Select one of the returned releases. We just pick the first one. ret = metadatas[0] @@ -395,10 +394,10 @@ def getTagList(self, number, mbdiscid): :rtype: dict """ - trackArtist = u'Unknown Artist' - releaseArtist = u'Unknown Artist' - disc = u'Unknown Disc' - title = u'Unknown Track' + trackArtist = 'Unknown Artist' + releaseArtist = 'Unknown Artist' + disc = 'Unknown Disc' + title = 'Unknown Track' if self.metadata: trackArtist = self.metadata.artist @@ -435,7 +434,7 @@ def getTagList(self, number, mbdiscid): tags['TITLE'] = title tags['ALBUM'] = disc - tags['TRACKNUMBER'] = u'%s' % number + tags['TRACKNUMBER'] = '%s' % number if self.metadata: if self.metadata.release is not None: @@ -573,7 +572,7 @@ def verifyImage(self, runner, table): def write_m3u(self, discname): m3uPath = common.truncate_filename(discname + '.m3u') with open(m3uPath, 'w') as f: - f.write(u'#EXTM3U\n'.encode('utf-8')) + f.write('#EXTM3U\n') for track in self.result.tracks: if not track.filename: # false positive htoa @@ -586,10 +585,10 @@ def write_m3u(self, discname): common.FRAMES_PER_SECOND) target_path = common.getRelativePath(track.filename, m3uPath) - u = u'#EXTINF:%d,%s\n' % (length, target_path) - f.write(u.encode('utf-8')) + u = '#EXTINF:%d,%s\n' % (length, target_path) + f.write(u) u = '%s\n' % target_path - f.write(u.encode('utf-8')) + f.write(u) def writeCue(self, discName): assert self.result.table.canCue() @@ -597,7 +596,7 @@ def writeCue(self, discName): logger.debug('write .cue file to %s', cuePath) handle = open(cuePath, 'w') # FIXME: do we always want utf-8 ? - handle.write(self.result.table.cue(cuePath).encode('utf-8')) + handle.write(self.result.table.cue(cuePath)) handle.close() self.cuePath = cuePath @@ -608,7 +607,7 @@ def writeLog(self, discName, txt_logger): logPath = common.truncate_filename(discName + '.log') handle = open(logPath, 'w') log = txt_logger.log(self.result) - handle.write(log.encode('utf-8')) + handle.write(log) handle.close() self.logPath = logPath diff --git a/whipper/extern/asyncsub.py b/whipper/extern/asyncsub.py index 4e4f9a8c..a2fe0210 100644 --- a/whipper/extern/asyncsub.py +++ b/whipper/extern/asyncsub.py @@ -11,7 +11,7 @@ PIPE = subprocess.PIPE -if subprocess.mswindows: +if sys.platform == 'win32': from win32file import ReadFile, WriteFile from win32pipe import PeekNamedPipe import msvcrt @@ -42,7 +42,7 @@ def _close(self, which): getattr(self, which).close() setattr(self, which, None) - if subprocess.mswindows: + if sys.platform == 'win32': def send(self, in_put): if not self.stdin: @@ -149,7 +149,7 @@ def recv_some(p, t=.1, e=1, tr=5, stderr=0): y.append(r) else: time.sleep(max((x - time.time()) / tr, 0)) - return ''.join(y) + return ''.join(x.decode() for x in y).encode() def send_all(p, data): diff --git a/whipper/extern/freedb.py b/whipper/extern/freedb.py index 6359a912..bd0ee548 100644 --- a/whipper/extern/freedb.py +++ b/whipper/extern/freedb.py @@ -53,15 +53,8 @@ def __repr__(self): "track_count", "playable_length"]])) - if sys.version_info[0] >= 3: - def __str__(self): - return self.__unicode__() - else: - def __str__(self): - return self.__unicode__().encode('ascii') - - def __unicode__(self): - return u"{:08X}".format(int(self)) + def __str__(self): + return "{:08X}".format(int(self)) def __int__(self): digit_sum_ = sum([digit_sum(o // 75) for o in self.offsets]) @@ -90,11 +83,11 @@ def perform_lookup(disc_id, freedb_server, freedb_port): query = freedb_command(freedb_server, freedb_port, - u"query", - *([disc_id.__unicode__(), - u"{:d}".format(disc_id.track_count)] + - [u"{:d}".format(o) for o in disc_id.offsets] + - [u"{:d}".format(disc_id.playable_length)])) + "query", + *([disc_id.__str__(), + "{:d}".format(disc_id.track_count)] + + ["{:d}".format(o) for o in disc_id.offsets] + + ["{:d}".format(disc_id.playable_length)])) line = next(query) response = RESPONSE.match(line) @@ -116,7 +109,7 @@ def perform_lookup(disc_id, freedb_server, freedb_port): elif (code == 211) or (code == 210): # multiple exact or inexact matches line = next(query) - while not line.startswith(u"."): + while not line.startswith("."): match = QUERY_RESULT.match(line) if match is not None: matches.append((match.group(1), @@ -140,7 +133,7 @@ def perform_lookup(disc_id, freedb_server, freedb_port): query = freedb_command(freedb_server, freedb_port, - u"read", + "read", category, disc_id) @@ -149,8 +142,8 @@ def perform_lookup(disc_id, freedb_server, freedb_port): # FIXME: check response code here freedb = {} line = next(query) - while not line.startswith(u"."): - if not line.startswith(u"#"): + while not line.startswith("."): + if not line.startswith("#"): entry = FREEDB_LINE.match(line) if entry is not None: if entry.group(1) in freedb: @@ -165,52 +158,38 @@ def perform_lookup(disc_id, freedb_server, freedb_port): def freedb_command(freedb_server, freedb_port, cmd, *args): """given a freedb_server string, freedb_port int, - command unicode string and argument unicode strings, - yields a list of Unicode strings""" + command string and argument strings, yields a list of strings""" - try: - from urllib.request import urlopen - from urllib.error import URLError - except ImportError: - from urllib2 import urlopen, URLError - try: - from urllib.parse import urlencode - except ImportError: - from urllib import urlencode + from urllib.error import URLError + from urllib.request import urlopen + from urllib.parse import urlencode from socket import getfqdn from whipper import __version__ as VERSION - from sys import version_info - - PY3 = version_info[0] >= 3 # some debug type checking - assert(isinstance(cmd, str if PY3 else unicode)) + assert(isinstance(cmd, str)) for arg in args: - assert(isinstance(arg, str if PY3 else unicode)) + assert(isinstance(arg, str)) POST = [] # generate query to post with arguments in specific order if len(args) > 0: - POST.append((u"cmd", u"cddb {} {}".format(cmd, " ".join(args)))) + POST.append(("cmd", "cddb {} {}".format(cmd, " ".join(args)))) else: - POST.append((u"cmd", u"cddb {}".format(cmd))) + POST.append(("cmd", "cddb {}".format(cmd))) POST.append( - (u"hello", - u"user {} {} {}".format( - getfqdn() if PY3 else getfqdn().decode("UTF-8", "replace"), - u"whipper", - VERSION if PY3 else VERSION.decode("ascii")))) + ("hello", + "user {} {} {}".format(getfqdn(), "whipper", VERSION))) - POST.append((u"proto", u"6")) + POST.append(("proto", "6")) try: # get Request object from post request = urlopen( "http://{}:{:d}/~cddb/cddb.cgi".format(freedb_server, freedb_port), - urlencode(POST).encode("UTF-8") if (version_info[0] >= 3) else - urlencode(POST)) + urlencode(POST).encode()) except URLError as e: raise ValueError(str(e)) try: diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index 3590c750..cdd572f9 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -18,7 +18,6 @@ # You should have received a copy of the GNU General Public License # along with whipper. If not, see . -from __future__ import print_function import logging import sys @@ -484,7 +483,7 @@ def run(self, task, verbose=None, skip=False): self._task.addListener(self) # only start the task after going into the mainloop, # otherwise the task might complete before we are in it - GLib.timeout_add(0L, self._startWrap, self._task) + GLib.timeout_add(0, self._startWrap, self._task) self.debug('run loop') self._loop.run() @@ -525,7 +524,7 @@ def c(): self.stopped(task) raise - GLib.timeout_add(int(delta * 1000L), c) + GLib.timeout_add(int(delta * 1000), c) # ITaskListener methods def progressed(self, task, value): diff --git a/whipper/image/cue.py b/whipper/image/cue.py index 1a49ca9d..a72e36d4 100644 --- a/whipper/image/cue.py +++ b/whipper/image/cue.py @@ -69,9 +69,9 @@ class CueFile(object): def __init__(self, path): """ - :type path: unicode + :type path: str """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path self._path = path self._rems = {} @@ -137,9 +137,9 @@ def parse(self): minutes = int(m.expand('\\2')) seconds = int(m.expand('\\3')) frames = int(m.expand('\\4')) - frameOffset = frames \ - + seconds * common.FRAMES_PER_SECOND \ - + minutes * common.FRAMES_PER_SECOND * 60 + frameOffset = int(frames + + seconds * common.FRAMES_PER_SECOND + + minutes * common.FRAMES_PER_SECOND * 60) logger.debug('found index %d of track %r in %r:%d', indexNumber, currentTrack, currentFile.path, @@ -182,7 +182,7 @@ def getRealPath(self, path): """ Translate the .cue's FILE to an existing path. - :type path: unicode + :type path: str """ return common.getRealPath(self._path, path) @@ -194,9 +194,9 @@ class File: def __init__(self, path, file_format): """ - :type path: unicode + :type path: str """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path self.path = path self.format = file_format diff --git a/whipper/image/image.py b/whipper/image/image.py index 83eb4b34..8d519839 100644 --- a/whipper/image/image.py +++ b/whipper/image/image.py @@ -43,10 +43,10 @@ class Image(object): def __init__(self, path): """ - :type path: unicode + :type path: str :param path: .cue path """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path self._path = path self.cue = cue.CueFile(path) @@ -62,7 +62,7 @@ def getRealPath(self, path): :param path: .cue path """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path return self.cue.getRealPath(path) @@ -130,7 +130,7 @@ def __init__(self, image): htoa = cue.table.tracks[0].indexes[0] track = cue.table.tracks[0] path = image.getRealPath(htoa.path) - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path logger.debug('schedule scan of audio length of %r', path) taskk = AudioLengthTask(path) self.addTask(taskk) @@ -145,7 +145,7 @@ def __init__(self, image): if length == -1: path = image.getRealPath(index.path) - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path logger.debug('schedule scan of audio length of %r', path) taskk = AudioLengthTask(path) self.addTask(taskk) @@ -192,7 +192,7 @@ def __init__(self, image, outdir): def add(index): path = image.getRealPath(index.path) - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path logger.debug('schedule encode of %r', path) root, _ = os.path.splitext(os.path.basename(path)) outpath = os.path.join(outdir, root + '.' + 'flac') diff --git a/whipper/image/table.py b/whipper/image/table.py index 57377a93..d1968352 100644 --- a/whipper/image/table.py +++ b/whipper/image/table.py @@ -23,8 +23,7 @@ """ import copy -import urllib -import urlparse +from urllib.parse import urlunparse, urlencode import whipper @@ -66,7 +65,7 @@ class Track: :vartype isrc: str :cvar cdtext: dictionary of CD Text information; :any:`see CDTEXT_KEYS` - :vartype cdtext: str -> unicode + :vartype cdtext: str :cvar pre_emphasis: whether track is pre-emphasised :vartype pre_emphasis: bool """ @@ -91,10 +90,10 @@ def __init__(self, number, audio=True, session=None): def index(self, number, absolute=None, path=None, relative=None, counter=None): """ - :type path: unicode or None + :type path: str or None """ if path is not None: - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path i = Index(number, absolute, path, relative, counter) self.indexes[number] = i @@ -133,7 +132,7 @@ class Index: """ :cvar counter: counter for the index source; distinguishes between the matching FILE lines in .cue files for example - :vartype path: unicode or None + :vartype path: str or None """ number = None absolute = None @@ -145,7 +144,7 @@ def __init__(self, number, absolute=None, path=None, relative=None, counter=None): if path is not None: - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path self.number = number self.absolute = absolute @@ -221,9 +220,10 @@ def getTrackEnd(self, number): # if on a session border, subtract the session leadin thisTrack = self.tracks[number - 1] nextTrack = self.tracks[number] - if nextTrack.session > thisTrack.session: - gap = self._getSessionGap(nextTrack.session) - end -= gap + if None not in [thisTrack.session, nextTrack.session]: + if nextTrack.session > thisTrack.session: + gap = self._getSessionGap(nextTrack.session) + end -= gap return end @@ -297,8 +297,8 @@ def getCDDBValues(self): # FIXME: we can't replace these calculations with the getFrameLength # call because the start and leadout in the algorithm get rounded # before making the difference - startSeconds = self.getTrackStart(1) / common.FRAMES_PER_SECOND - leadoutSeconds = leadout / common.FRAMES_PER_SECOND + startSeconds = self.getTrackStart(1) // common.FRAMES_PER_SECOND + leadoutSeconds = leadout // common.FRAMES_PER_SECOND t = leadoutSeconds - startSeconds # durationFrames = self.getFrameLength(data=True) # duration = durationFrames / common.FRAMES_PER_SECOND @@ -348,12 +348,12 @@ def getMusicBrainzDiscId(self): sha = sha1() # number of first track - sha.update("%02X" % values[0]) + sha.update(("%02X" % values[0]).encode()) # number of last track - sha.update("%02X" % values[1]) + sha.update(("%02X" % values[1]).encode()) - sha.update("%08X" % values[2]) + sha.update(("%08X" % values[2]).encode()) # offsets of tracks for i in range(1, 100): @@ -361,7 +361,7 @@ def getMusicBrainzDiscId(self): offset = values[2 + i] except IndexError: offset = 0 - sha.update("%08X" % offset) + sha.update(("%08X" % offset).encode()) digest = sha.digest() assert len(digest) == 20, \ @@ -372,10 +372,10 @@ def getMusicBrainzDiscId(self): # (Rob) used ., _, and - # base64 altchars specify replacements for + and / - result = base64.b64encode(digest, '._') + result = base64.b64encode(digest, b'._').decode() # now replace = - result = "-".join(result.split("=")) + result = result.replace("=", "-") assert len(result) == 28, \ "Result should be 28 characters, not %d" % len(result) @@ -389,13 +389,13 @@ def getMusicBrainzSubmitURL(self): discid = self.getMusicBrainzDiscId() values = self._getMusicBrainzValues() - query = urllib.urlencode({ + query = urlencode({ 'id': discid, 'toc': ' '.join([str(v) for v in values]), 'tracks': self.getAudioTracks(), }) - return urlparse.urlunparse(( + return urlunparse(( 'https', host, '/cdtoc/attach', '', query, '')) def getFrameLength(self, data=False): @@ -477,7 +477,7 @@ def cue(self, cuePath='', program='whipper'): Dump our internal representation to a .cue file content. - :rtype: unicode + :rtype: str """ logger.debug('generating .cue for cuePath %r', cuePath) diff --git a/whipper/image/toc.py b/whipper/image/toc.py index da175c16..d3cc3ccf 100644 --- a/whipper/image/toc.py +++ b/whipper/image/toc.py @@ -138,9 +138,9 @@ class TocFile(object): def __init__(self, path): """ - :type path: unicode + :type path: str """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not string" % path self._path = path self._messages = [] self.table = table.Table() @@ -202,7 +202,7 @@ def parse(self): # usually, value is encoded with octal escapes and in latin-1 # FIXME: other encodings are possible, does cdrdao handle # them ? - value = value.decode('string-escape').decode('latin-1') + value = value.encode().decode('unicode_escape') if key in table.CDTEXT_FIELDS: # FIXME: consider ISRC separate for now, but this # is a limitation of our parser approach @@ -412,7 +412,7 @@ def getRealPath(self, path): """ Translate the .toc's FILE to an existing path. - :type path: unicode + :type path: str """ return common.getRealPath(self._path, path) @@ -424,12 +424,12 @@ class File: def __init__(self, path, start, length): """ - :type path: unicode + :type path: str :type start: int :param start: starting point for the track in this file, in frames :param length: length for the track in this file, in frames """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path self.path = path self.start = start diff --git a/whipper/program/arc.py b/whipper/program/arc.py index 9e6134a5..3f207631 100644 --- a/whipper/program/arc.py +++ b/whipper/program/arc.py @@ -2,4 +2,4 @@ def accuraterip_checksum(f, track_number, total_tracks): - return accuraterip.compute(f.encode('utf-8'), track_number, total_tracks) + return accuraterip.compute(f, track_number, total_tracks) diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index 160646eb..5478311d 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -220,7 +220,7 @@ def __init__(self, path, table, start, stop, overread, offset=0, Read the given track. :param path: where to store the ripped track - :type path: unicode + :type path: str :param table: table of contents of CD :type table: table.Table :param start: first frame to rip @@ -236,7 +236,7 @@ def __init__(self, path, table, start, stop, overread, offset=0, :param what: a string representing what's being read; e.g. Track :type what: str """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path self.path = path self._table = table @@ -314,7 +314,7 @@ def _read(self, runner): self.schedule(0.01, self._read, runner) return - self._buffer += ret + self._buffer += ret.decode() # parse buffer into lines if possible, and parse them if "\n" in self._buffer: @@ -452,7 +452,6 @@ def __init__(self, path, table, start, stop, overread, offset=0, logger.debug('read and verify with taglist %r', taglist) # FIXME: choose a dir on the same disk/dir as the final path fd, tmppath = tempfile.mkstemp(suffix='.whipper.wav') - tmppath = unicode(tmppath) os.fchmod(fd, 0644) os.close(fd) self._tmpwavpath = tmppath @@ -472,13 +471,13 @@ def __init__(self, path, table, start, stop, overread, offset=0, # encode to the final path + '.part' try: - tmpoutpath = path + u'.part' + tmpoutpath = path + '.part' open(tmpoutpath, 'wb').close() except IOError as e: if errno.ENAMETOOLONG != e.errno: raise path = common.truncate_filename(common.shrinkPath(path)) - tmpoutpath = common.truncate_filename(path + u'.part') + tmpoutpath = common.truncate_filename(path + '.part') open(tmpoutpath, 'wb').close() self._tmppath = tmpoutpath self.path = path @@ -597,7 +596,7 @@ def readbyteserr(self, bytes_stderr): def done(self): if self.cwd: shutil.rmtree(self.cwd) - output = "".join(self._output) + output = "".join(o.decode() for o in self._output) m = _OK_RE.search(output) self.defeatsCache = bool(m) diff --git a/whipper/program/cdrdao.py b/whipper/program/cdrdao.py index 9b36adab..365c5ade 100644 --- a/whipper/program/cdrdao.py +++ b/whipper/program/cdrdao.py @@ -84,7 +84,7 @@ def __init__(self, device, fast_toc=False, toc_path=None): self._parser = ProgressParser() self.fd, self.tocfile = tempfile.mkstemp( - suffix=u'.cdrdao.read-toc.whipper.task') + suffix='.cdrdao.read-toc.whipper.task') def start(self, runner): task.Task.start(self, runner) @@ -112,7 +112,7 @@ def _read(self, runner): return self.schedule(0.01, self._read, runner) return - self._buffer += ret + self._buffer += ret.decode() # parse buffer into lines if possible, and parse them if "\n" in self._buffer: @@ -168,7 +168,7 @@ def DetectCdr(device): cmd = [CDRDAO, 'disk-info', '-v1', '--device', device] logger.debug("executing %r", cmd) p = Popen(cmd, stdout=PIPE, stderr=PIPE) - return 'CD-R medium : n/a' not in p.stdout.read() + return 'CD-R medium : n/a' not in p.stdout.read().decode() def version(): diff --git a/whipper/program/soxi.py b/whipper/program/soxi.py index 86a18235..387dfcc7 100644 --- a/whipper/program/soxi.py +++ b/whipper/program/soxi.py @@ -21,11 +21,11 @@ class AudioLengthTask(ctask.PopenTask): def __init__(self, path): """ - :type path: unicode + :type path: str """ - assert isinstance(path, unicode), "%r is not unicode" % path + assert isinstance(path, str), "%r is not str" % path - self.logName = os.path.basename(path).encode('utf-8') + self.logName = os.path.basename(path) self.command = [SOXI, '-s', path] @@ -47,4 +47,4 @@ def failed(self): def done(self): if self._error: logger.warning("soxi reported on stderr: %s", "".join(self._error)) - self.length = int("".join(self._output)) + self.length = int("".join(o.decode() for o in self._output)) diff --git a/whipper/test/common.py b/whipper/test/common.py index a35b96e5..b1ba2e71 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -83,7 +83,7 @@ class UnicodeTestMixin: # A helper mixin to skip tests if we're not in a UTF-8 locale try: - os.stat(u'whipper.test.B\xeate Noire.empty') + os.stat('whipper.test.B\xeate Noire.empty') except UnicodeEncodeError: skip = 'No UTF-8 locale' except OSError: diff --git a/whipper/test/test_command_mblookup.py b/whipper/test/test_command_mblookup.py index c358ca83..ee8d6644 100644 --- a/whipper/test/test_command_mblookup.py +++ b/whipper/test/test_command_mblookup.py @@ -1,5 +1,5 @@ # vi:si:et:sw=4:sts=4:ts=4:set fileencoding=utf-8 -u"""Tests for whipper.command.mblookup""" +"""Tests for whipper.command.mblookup""" import os import pickle @@ -9,22 +9,22 @@ class MBLookupTestCase(unittest.TestCase): - u"""Test cases for whipper.command.mblookup.MBLookup""" + """Test cases for whipper.command.mblookup.MBLookup""" @staticmethod def _mock_musicbrainz(discid, country=None, record=False): - u"""Mock function for whipper.common.mbngs.musicbrainz function.""" - filename = u"whipper.discid.{}.pickle".format(discid) + """Mock function for whipper.common.mbngs.musicbrainz function.""" + filename = "whipper.discid.{}.pickle".format(discid) path = os.path.join(os.path.dirname(__file__), filename) with open(path) as p: return pickle.load(p) def testMissingReleaseType(self): - u"""Test that lookup for release without a type set doesn't fail.""" + """Test that lookup for release without a type set doesn't fail.""" # Using: Gustafsson, Österberg & Cowle - What's Up? 8 (disc 4) # https://musicbrainz.org/release/d8e6153a-2c47-4804-9d73-0aac1081c3b1 mblookup.musicbrainz = self._mock_musicbrainz - discid = u"xu338_M8WukSRi0J.KTlDoflB8Y-" + discid = "xu338_M8WukSRi0J.KTlDoflB8Y-" # https://musicbrainz.org/cdtoc/xu338_M8WukSRi0J.KTlDoflB8Y- - lookup = mblookup.MBLookup([discid], u'whipper mblookup', None) + lookup = mblookup.MBLookup([discid], 'whipper mblookup', None) lookup.do() diff --git a/whipper/test/test_common_common.py b/whipper/test/test_common_common.py index a6f6e57a..3cf1c40c 100644 --- a/whipper/test/test_common_common.py +++ b/whipper/test/test_common_common.py @@ -12,7 +12,7 @@ class ShrinkTestCase(tcommon.TestCase): def testSufjan(self): - path = (u'whipper/Sufjan Stevens - Illinois/02. Sufjan Stevens - ' + path = ('whipper/Sufjan Stevens - Illinois/02. Sufjan Stevens - ' 'The Black Hawk War, or, How to Demolish an Entire ' 'Civilization and Still Feel Good About Yourself in the ' 'Morning, or, We Apologize for the Inconvenience but ' @@ -52,7 +52,7 @@ def testRelativeOutputDirectory(self): class GetRealPathTestCase(tcommon.TestCase): def testRealWithBackslash(self): - fd, path = tempfile.mkstemp(suffix=u'back\\slash.flac') + fd, path = tempfile.mkstemp(suffix='back\\slash.flac') refPath = os.path.join(os.path.dirname(path), 'fake.cue') self.assertEqual(common.getRealPath(refPath, path), path) diff --git a/whipper/test/test_common_config.py b/whipper/test/test_common_config.py index 0653aada..d0bcb441 100644 --- a/whipper/test/test_common_config.py +++ b/whipper/test/test_common_config.py @@ -12,7 +12,7 @@ class ConfigTestCase(tcommon.TestCase): def setUp(self): - fd, self._path = tempfile.mkstemp(suffix=u'.whipper.test.config') + fd, self._path = tempfile.mkstemp(suffix='.whipper.test.config') os.close(fd) self._config = config.Config(self._path) diff --git a/whipper/test/test_common_mbngs.py b/whipper/test/test_common_mbngs.py index 6cc89d8d..b99a1bcc 100644 --- a/whipper/test/test_common_mbngs.py +++ b/whipper/test/test_common_mbngs.py @@ -37,10 +37,10 @@ def test2MeterSessies10(self): metadata = mbngs._getMetadata(response['release'], discid) - self.assertEqual(metadata.artist, u'Various Artists') - self.assertEqual(metadata.release, u'2001-10-15') + self.assertEqual(metadata.artist, 'Various Artists') + self.assertEqual(metadata.release, '2001-10-15') self.assertEqual(metadata.mbidArtist, - [u'89ad4ac3-39f7-470e-963a-56509c546377']) + ['89ad4ac3-39f7-470e-963a-56509c546377']) self.assertEqual(len(metadata.tracks), 18) @@ -48,11 +48,11 @@ def test2MeterSessies10(self): self.assertEqual(track16.artist, 'Tom Jones & Stereophonics') self.assertEqual(track16.mbidArtist, [ - u'57c6f649-6cde-48a7-8114-2a200247601a', - u'0bfba3d3-6a04-4779-bb0a-df07df5b0558', + '57c6f649-6cde-48a7-8114-2a200247601a', + '0bfba3d3-6a04-4779-bb0a-df07df5b0558', ]) self.assertEqual(track16.sortName, - u'Jones, Tom & Stereophonics') + 'Jones, Tom & Stereophonics') def testBalladOfTheBrokenSeas(self): # various artists disc @@ -65,26 +65,26 @@ def testBalladOfTheBrokenSeas(self): metadata = mbngs._getMetadata(response['release'], discid) - self.assertEqual(metadata.artist, u'Isobel Campbell & Mark Lanegan') + self.assertEqual(metadata.artist, 'Isobel Campbell & Mark Lanegan') self.assertEqual(metadata.sortName, - u'Campbell, Isobel & Lanegan, Mark') - self.assertEqual(metadata.release, u'2006-01-30') + 'Campbell, Isobel & Lanegan, Mark') + self.assertEqual(metadata.release, '2006-01-30') self.assertEqual(metadata.mbidArtist, [ - u'd51f3a15-12a2-41a0-acfa-33b5eae71164', - u'a9126556-f555-4920-9617-6e013f8228a7', + 'd51f3a15-12a2-41a0-acfa-33b5eae71164', + 'a9126556-f555-4920-9617-6e013f8228a7', ]) self.assertEqual(len(metadata.tracks), 12) track12 = metadata.tracks[11] - self.assertEqual(track12.artist, u'Isobel Campbell & Mark Lanegan') + self.assertEqual(track12.artist, 'Isobel Campbell & Mark Lanegan') self.assertEqual(track12.sortName, - u'Campbell, Isobel' + 'Campbell, Isobel' ' & Lanegan, Mark') self.assertEqual(track12.mbidArtist, [ - u'd51f3a15-12a2-41a0-acfa-33b5eae71164', - u'a9126556-f555-4920-9617-6e013f8228a7', + 'd51f3a15-12a2-41a0-acfa-33b5eae71164', + 'a9126556-f555-4920-9617-6e013f8228a7', ]) def testMalaInCuba(self): @@ -99,23 +99,23 @@ def testMalaInCuba(self): metadata = mbngs._getMetadata(response['release'], discid) - self.assertEqual(metadata.artist, u'Mala') - self.assertEqual(metadata.sortName, u'Mala') - self.assertEqual(metadata.release, u'2012-09-17') + self.assertEqual(metadata.artist, 'Mala') + self.assertEqual(metadata.sortName, 'Mala') + self.assertEqual(metadata.release, '2012-09-17') self.assertEqual(metadata.mbidArtist, - [u'09f221eb-c97e-4da5-ac22-d7ab7c555bbb']) + ['09f221eb-c97e-4da5-ac22-d7ab7c555bbb']) self.assertEqual(len(metadata.tracks), 14) track6 = metadata.tracks[5] - self.assertEqual(track6.artist, u'Mala feat. Dreiser & Sexto Sentido') + self.assertEqual(track6.artist, 'Mala feat. Dreiser & Sexto Sentido') self.assertEqual(track6.sortName, - u'Mala feat. Dreiser & Sexto Sentido') + 'Mala feat. Dreiser & Sexto Sentido') self.assertEqual(track6.mbidArtist, [ - u'09f221eb-c97e-4da5-ac22-d7ab7c555bbb', - u'ec07a209-55ff-4084-bc41-9d4d1764e075', - u'f626b92e-07b1-4a19-ad13-c09d690db66c', + '09f221eb-c97e-4da5-ac22-d7ab7c555bbb', + 'ec07a209-55ff-4084-bc41-9d4d1764e075', + 'f626b92e-07b1-4a19-ad13-c09d690db66c', ]) def testUnknownArtist(self): @@ -135,28 +135,28 @@ def testUnknownArtist(self): discid = "RhrwgVb0hZNkabQCw1dZIhdbMFg-" metadata = mbngs._getMetadata(response['release'], discid) - self.assertEqual(metadata.artist, u'CunninLynguists') - self.assertEqual(metadata.release, u'2003') + self.assertEqual(metadata.artist, 'CunninLynguists') + self.assertEqual(metadata.release, '2003') self.assertEqual(metadata.mbidArtist, - [u'69c4cc43-8163-41c5-ac81-30946d27bb69']) + ['69c4cc43-8163-41c5-ac81-30946d27bb69']) self.assertEqual(len(metadata.tracks), 30) track8 = metadata.tracks[7] - self.assertEqual(track8.artist, u'???') - self.assertEqual(track8.sortName, u'[unknown]') + self.assertEqual(track8.artist, '???') + self.assertEqual(track8.sortName, '[unknown]') self.assertEqual(track8.mbidArtist, - [u'125ec42a-7229-4250-afc5-e057484327fe']) + ['125ec42a-7229-4250-afc5-e057484327fe']) track9 = metadata.tracks[8] - self.assertEqual(track9.artist, u'CunninLynguists feat. Tonedeff') + self.assertEqual(track9.artist, 'CunninLynguists feat. Tonedeff') self.assertEqual(track9.sortName, - u'CunninLynguists feat. Tonedeff') + 'CunninLynguists feat. Tonedeff') self.assertEqual(track9.mbidArtist, [ - u'69c4cc43-8163-41c5-ac81-30946d27bb69', - u'b3869d83-9fb5-4eac-b5ca-2d155fcbee12' + '69c4cc43-8163-41c5-ac81-30946d27bb69', + 'b3869d83-9fb5-4eac-b5ca-2d155fcbee12' ]) def testNenaAndKimWildSingle(self): @@ -172,40 +172,40 @@ def testNenaAndKimWildSingle(self): discid = "X2c2IQ5vUy5x6Jh7Xi_DGHtA1X8-" metadata = mbngs._getMetadata(response['release'], discid) - self.assertEqual(metadata.artist, u'Nena & Kim Wilde') - self.assertEqual(metadata.release, u'2003-05-19') + self.assertEqual(metadata.artist, 'Nena & Kim Wilde') + self.assertEqual(metadata.release, '2003-05-19') self.assertEqual(metadata.mbidArtist, [ - u'38bfaa7f-ee98-48cb-acd0-946d7aeecd76', - u'4b462375-c508-432a-8c88-ceeec38b16ae', + '38bfaa7f-ee98-48cb-acd0-946d7aeecd76', + '4b462375-c508-432a-8c88-ceeec38b16ae', ]) self.assertEqual(len(metadata.tracks), 4) track1 = metadata.tracks[0] - self.assertEqual(track1.artist, u'Nena & Kim Wilde') - self.assertEqual(track1.sortName, u'Nena & Wilde, Kim') + self.assertEqual(track1.artist, 'Nena & Kim Wilde') + self.assertEqual(track1.sortName, 'Nena & Wilde, Kim') self.assertEqual(track1.mbidArtist, [ - u'38bfaa7f-ee98-48cb-acd0-946d7aeecd76', - u'4b462375-c508-432a-8c88-ceeec38b16ae', + '38bfaa7f-ee98-48cb-acd0-946d7aeecd76', + '4b462375-c508-432a-8c88-ceeec38b16ae', ]) self.assertEqual(track1.mbid, - u'1cc96e78-28ed-3820-b0b6-614c35b121ac') + '1cc96e78-28ed-3820-b0b6-614c35b121ac') self.assertEqual(track1.mbidRecording, - u'fde5622c-ce23-4ebb-975d-51d4a926f901') + 'fde5622c-ce23-4ebb-975d-51d4a926f901') track2 = metadata.tracks[1] - self.assertEqual(track2.artist, u'Nena & Kim Wilde') - self.assertEqual(track2.sortName, u'Nena & Wilde, Kim') + self.assertEqual(track2.artist, 'Nena & Kim Wilde') + self.assertEqual(track2.sortName, 'Nena & Wilde, Kim') self.assertEqual(track2.mbidArtist, [ - u'38bfaa7f-ee98-48cb-acd0-946d7aeecd76', - u'4b462375-c508-432a-8c88-ceeec38b16ae', + '38bfaa7f-ee98-48cb-acd0-946d7aeecd76', + '4b462375-c508-432a-8c88-ceeec38b16ae', ]) self.assertEqual(track2.mbid, - u'f16db4bf-9a34-3d5a-a975-c9375ab7a2ca') + 'f16db4bf-9a34-3d5a-a975-c9375ab7a2ca') self.assertEqual(track2.mbidRecording, - u'5f19758e-7421-4c71-a599-9a9575d8e1b0') + '5f19758e-7421-4c71-a599-9a9575d8e1b0') def testMissingReleaseGroupType(self): """Check that whipper doesn't break if there's no type.""" @@ -233,37 +233,37 @@ def testAllAvailableMetadata(self): discid = "cHW1Uutl_kyWNaLJsLmTGTe4rnE-" metadata = mbngs._getMetadata(response['release'], discid) - self.assertEqual(metadata.artist, u'David Rovics') - self.assertEqual(metadata.sortName, u'Rovics, David') + self.assertEqual(metadata.artist, 'David Rovics') + self.assertEqual(metadata.sortName, 'Rovics, David') self.assertFalse(metadata.various) self.assertIsInstance(metadata.tracks, list) - self.assertEqual(metadata.release, u'2015') - self.assertEqual(metadata.releaseTitle, u'The Other Side') - self.assertEqual(metadata.releaseType, u'Album') + self.assertEqual(metadata.release, '2015') + self.assertEqual(metadata.releaseTitle, 'The Other Side') + self.assertEqual(metadata.releaseType, 'Album') self.assertEqual(metadata.mbid, - u'6109ceed-7e21-490b-b5ad-3a66b4e4cfbb') + '6109ceed-7e21-490b-b5ad-3a66b4e4cfbb') self.assertEqual(metadata.mbidReleaseGroup, - u'99850b41-a06e-4fb8-992c-75c191a77803') + '99850b41-a06e-4fb8-992c-75c191a77803') self.assertEqual(metadata.mbidArtist, - [u'4d56eb9f-13b3-4f05-9db7-50195378d49f']) + ['4d56eb9f-13b3-4f05-9db7-50195378d49f']) self.assertEqual(metadata.url, - u'https://musicbrainz.org/release' + 'https://musicbrainz.org/release' '/6109ceed-7e21-490b-b5ad-3a66b4e4cfbb') - self.assertEqual(metadata.catalogNumber, u'[none]') - self.assertEqual(metadata.barcode, u'700261430249') + self.assertEqual(metadata.catalogNumber, '[none]') + self.assertEqual(metadata.barcode, '700261430249') self.assertEqual(len(metadata.tracks), 16) track1 = metadata.tracks[0] - self.assertEqual(track1.artist, u'David Rovics') - self.assertEqual(track1.title, u'Waiting for the Hurricane') + self.assertEqual(track1.artist, 'David Rovics') + self.assertEqual(track1.title, 'Waiting for the Hurricane') self.assertEqual(track1.duration, 176320) self.assertEqual(track1.mbid, - u'4116eea3-b9c2-452a-8d63-92f1e585b225') - self.assertEqual(track1.sortName, u'Rovics, David') + '4116eea3-b9c2-452a-8d63-92f1e585b225') + self.assertEqual(track1.sortName, 'Rovics, David') self.assertEqual(track1.mbidArtist, - [u'4d56eb9f-13b3-4f05-9db7-50195378d49f']) + ['4d56eb9f-13b3-4f05-9db7-50195378d49f']) self.assertEqual(track1.mbidRecording, - u'b191794d-b7c6-4d6f-971e-0a543959b5ad') + 'b191794d-b7c6-4d6f-971e-0a543959b5ad') self.assertEqual(track1.mbidWorks, - [u'90d5be68-0b29-45a3-ba01-c27ad78e3625']) + ['90d5be68-0b29-45a3-ba01-c27ad78e3625']) diff --git a/whipper/test/test_common_path.py b/whipper/test/test_common_path.py index 41b7cbeb..0f59678a 100644 --- a/whipper/test/test_common_path.py +++ b/whipper/test/test_common_path.py @@ -12,19 +12,19 @@ def setUp(self): self._filter = path.PathFilter(special=True) def testSlash(self): - part = u'A Charm/A Blade' - self.assertEqual(self._filter.filter(part), u'A Charm-A Blade') + part = 'A Charm/A Blade' + self.assertEqual(self._filter.filter(part), 'A Charm-A Blade') def testFat(self): - part = u'A Word: F**k you?' - self.assertEqual(self._filter.filter(part), u'A Word - F__k you_') + part = 'A Word: F**k you?' + self.assertEqual(self._filter.filter(part), 'A Word - F__k you_') def testSpecial(self): - part = u'<<< $&*!\' "()`{}[]spaceship>>>' + part = '<<< $&*!\' "()`{}[]spaceship>>>' self.assertEqual(self._filter.filter(part), - u'___ _____ ________spaceship___') + '___ _____ ________spaceship___') def testGreatest(self): - part = u'Greatest Ever! Soul: The Definitive Collection' + part = 'Greatest Ever! Soul: The Definitive Collection' self.assertEqual(self._filter.filter(part), - u'Greatest Ever_ Soul - The Definitive Collection') + 'Greatest Ever_ Soul - The Definitive Collection') diff --git a/whipper/test/test_common_program.py b/whipper/test/test_common_program.py index 4fba9874..36cc7a62 100644 --- a/whipper/test/test_common_program.py +++ b/whipper/test/test_common_program.py @@ -13,10 +13,10 @@ class PathTestCase(unittest.TestCase): def testStandardTemplateEmpty(self): prog = program.Program(config.Config()) - path = prog.getPath(u'/tmp', DEFAULT_DISC_TEMPLATE, + path = prog.getPath('/tmp', DEFAULT_DISC_TEMPLATE, 'mbdiscid', None) - self.assertEqual(path, (u'/tmp/unknown/Unknown Artist - mbdiscid/' - u'Unknown Artist - mbdiscid')) + self.assertEqual(path, ('/tmp/unknown/Unknown Artist - mbdiscid/' + 'Unknown Artist - mbdiscid')) def testStandardTemplateFilled(self): prog = program.Program(config.Config()) @@ -24,10 +24,10 @@ def testStandardTemplateFilled(self): md.artist = md.sortName = 'Jeff Buckley' md.title = 'Grace' - path = prog.getPath(u'/tmp', DEFAULT_DISC_TEMPLATE, + path = prog.getPath('/tmp', DEFAULT_DISC_TEMPLATE, 'mbdiscid', md, 0) - self.assertEqual(path, (u'/tmp/unknown/Jeff Buckley - Grace/' - u'Jeff Buckley - Grace')) + self.assertEqual(path, ('/tmp/unknown/Jeff Buckley - Grace/' + 'Jeff Buckley - Grace')) def testIssue66TemplateFilled(self): prog = program.Program(config.Config()) @@ -35,6 +35,6 @@ def testIssue66TemplateFilled(self): md.artist = md.sortName = 'Jeff Buckley' md.title = 'Grace' - path = prog.getPath(u'/tmp', u'%A/%d', 'mbdiscid', md, 0) + path = prog.getPath('/tmp', '%A/%d', 'mbdiscid', md, 0) self.assertEqual(path, - u'/tmp/Jeff Buckley/Grace') + '/tmp/Jeff Buckley/Grace') diff --git a/whipper/test/test_image_cue.py b/whipper/test/test_image_cue.py index d73333fe..ea4066af 100644 --- a/whipper/test/test_image_cue.py +++ b/whipper/test/test_image_cue.py @@ -16,7 +16,7 @@ class KingsSingleTestCase(unittest.TestCase): def setUp(self): self.cue = cue.CueFile(os.path.join(os.path.dirname(__file__), - u'kings-single.cue')) + 'kings-single.cue')) self.cue.parse() self.assertEqual(len(self.cue.table.tracks), 11) @@ -32,7 +32,7 @@ class KingsSeparateTestCase(unittest.TestCase): def setUp(self): self.cue = cue.CueFile(os.path.join(os.path.dirname(__file__), - u'kings-separate.cue')) + 'kings-separate.cue')) self.cue.parse() self.assertEqual(len(self.cue.table.tracks), 11) @@ -48,7 +48,7 @@ class KanyeMixedTestCase(unittest.TestCase): def setUp(self): self.cue = cue.CueFile(os.path.join(os.path.dirname(__file__), - u'kanye.cue')) + 'kanye.cue')) self.cue.parse() self.assertEqual(len(self.cue.table.tracks), 13) @@ -61,24 +61,24 @@ class WriteCueFileTestCase(unittest.TestCase): @staticmethod def testWrite(): - fd, path = tempfile.mkstemp(suffix=u'.whipper.test.cue') + fd, path = tempfile.mkstemp(suffix='.whipper.test.cue') os.close(fd) it = table.Table() t = table.Track(1) - t.index(1, absolute=0, path=u'track01.wav', relative=0, counter=1) + t.index(1, absolute=0, path='track01.wav', relative=0, counter=1) it.tracks.append(t) t = table.Track(2) - t.index(0, absolute=1000, path=u'track01.wav', + t.index(0, absolute=1000, path='track01.wav', relative=1000, counter=1) - t.index(1, absolute=2000, path=u'track02.wav', relative=0, counter=2) + t.index(1, absolute=2000, path='track02.wav', relative=0, counter=2) it.tracks.append(t) it.absolutize() it.leadout = 3000 - common.diffStrings(u"""REM DISCID 0C002802 + common.diffStrings("""REM DISCID 0C002802 REM COMMENT "whipper %s" FILE "track01.wav" WAVE TRACK 01 AUDIO diff --git a/whipper/test/test_image_toc.py b/whipper/test/test_image_toc.py index 4786662a..34293d19 100644 --- a/whipper/test/test_image_toc.py +++ b/whipper/test/test_image_toc.py @@ -14,7 +14,7 @@ class CureTestCase(common.TestCase): def setUp(self): - self.path = os.path.join(os.path.dirname(__file__), u'cure.toc') + self.path = os.path.join(os.path.dirname(__file__), 'cure.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 13) @@ -93,8 +93,8 @@ def testConvertCue(self): '3/c/4/dBAR-013-0019d4c3-00fe8924-b90c650d.bin') def testGetRealPath(self): - self.assertRaises(KeyError, self.toc.getRealPath, u'track01.wav') - (fd, path) = tempfile.mkstemp(suffix=u'.whipper.test.wav') + self.assertRaises(KeyError, self.toc.getRealPath, 'track01.wav') + (fd, path) = tempfile.mkstemp(suffix='.whipper.test.wav') self.assertEqual(self.toc.getRealPath(path), path) winpath = path.replace('/', '\\') @@ -108,7 +108,7 @@ def testGetRealPath(self): class BlocTestCase(common.TestCase): def setUp(self): - self.path = os.path.join(os.path.dirname(__file__), u'bloc.toc') + self.path = os.path.join(os.path.dirname(__file__), 'bloc.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 13) @@ -173,7 +173,7 @@ def testAccurateRip(self): class BreedersTestCase(common.TestCase): def setUp(self): - self.path = os.path.join(os.path.dirname(__file__), u'breeders.toc') + self.path = os.path.join(os.path.dirname(__file__), 'breeders.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 13) @@ -200,7 +200,7 @@ def testConvertCue(self): class LadyhawkeTestCase(common.TestCase): def setUp(self): - self.path = os.path.join(os.path.dirname(__file__), u'ladyhawke.toc') + self.path = os.path.join(os.path.dirname(__file__), 'ladyhawke.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 13) @@ -237,13 +237,13 @@ class CapitalMergeTestCase(common.TestCase): def setUp(self): self.toc1 = toc.TocFile(os.path.join(os.path.dirname(__file__), - u'capital.1.toc')) + 'capital.1.toc')) self.toc1.parse() self.assertEqual(len(self.toc1.table.tracks), 11) self.assertTrue(self.toc1.table.tracks[-1].audio) self.toc2 = toc.TocFile(os.path.join(os.path.dirname(__file__), - u'capital.2.toc')) + 'capital.2.toc')) self.toc2.parse() self.assertEqual(len(self.toc2.table.tracks), 1) self.assertFalse(self.toc2.table.tracks[-1].audio) @@ -278,8 +278,8 @@ def setUp(self): # we copy the normal non-utf8 filename to a utf-8 filename # in this test because builds with LANG=C fail if we include # utf-8 filenames in the dist - path = u'Jos\xe9Gonz\xe1lez.toc' - self._performer = u'Jos\xe9 Gonz\xe1lez' + path = 'Jos\xe9Gonz\xe1lez.toc' + self._performer = 'Jos\xe9 Gonz\xe1lez' source = os.path.join(os.path.dirname(__file__), 'jose.toc') (fd, self.dest) = tempfile.mkstemp(suffix=path) os.close(fd) @@ -311,7 +311,7 @@ def testGetTrackPerformer(self): class TOTBLTestCase(common.TestCase): def setUp(self): - self.path = os.path.join(os.path.dirname(__file__), u'totbl.fast.toc') + self.path = os.path.join(os.path.dirname(__file__), 'totbl.fast.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 11) @@ -324,7 +324,7 @@ class GentlemenTestCase(common.TestCase): def setUp(self): self.path = os.path.join(os.path.dirname(__file__), - u'gentlemen.fast.toc') + 'gentlemen.fast.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEquals(len(self.toc.table.tracks), 11) @@ -341,7 +341,7 @@ class StrokesTestCase(common.TestCase): def setUp(self): self.path = os.path.join(os.path.dirname(__file__), - u'strokes-someday.toc') + 'strokes-someday.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 1) @@ -358,7 +358,7 @@ def testIndexes(self): self.assertEqual(i1.relative, 0) self.assertEqual(i1.absolute, 1) self.assertEqual(i1.counter, 1) - self.assertEqual(i1.path, u'data.wav') + self.assertEqual(i1.path, 'data.wav') cue = self._filterCue(self.toc.table.cue()) ref = self._filterCue( @@ -400,7 +400,7 @@ def _filterCue(output): class SurferRosaTestCase(common.TestCase): def setUp(self): - self.path = os.path.join(os.path.dirname(__file__), u'surferrosa.toc') + self.path = os.path.join(os.path.dirname(__file__), 'surferrosa.toc') self.toc = toc.TocFile(self.path) self.toc.parse() self.assertEqual(len(self.toc.table.tracks), 21) diff --git a/whipper/test/test_program_soxi.py b/whipper/test/test_program_soxi.py index 4ebf62fa..32f3e23a 100644 --- a/whipper/test/test_program_soxi.py +++ b/whipper/test/test_program_soxi.py @@ -8,7 +8,7 @@ from whipper.program.soxi import AudioLengthTask from whipper.test import common as tcommon -base_track_file = os.path.join(os.path.dirname(__file__), u'track.flac') +base_track_file = os.path.join(os.path.dirname(__file__), 'track.flac') base_track_length = 10 * common.SAMPLES_PER_FRAME @@ -39,26 +39,18 @@ def _testSuffix(self, suffix): class NormalAudioLengthPathTestCase(AudioLengthPathTestCase): def testSingleQuote(self): - self._testSuffix(u"whipper.test.Guns 'N Roses.flac") + self._testSuffix("whipper.test.Guns 'N Roses.flac") def testDoubleQuote(self): # This test makes sure we can checksum files with double quote in # their name - self._testSuffix(u'whipper.test.12" edit.flac') - - -class UnicodeAudioLengthPathTestCase(AudioLengthPathTestCase, - tcommon.UnicodeTestMixin): - - def testUnicodePath(self): - # this test makes sure we can checksum a unicode path - self._testSuffix(u'whipper.test.B\xeate Noire.empty.flac') + self._testSuffix('whipper.test.12" edit.flac') class AbsentFileAudioLengthPathTestCase(AudioLengthPathTestCase): def testAbsentFile(self): tempdir = tempfile.mkdtemp() - path = os.path.join(tempdir, u"nonexistent.flac") + path = os.path.join(tempdir, "nonexistent.flac") t = AudioLengthTask(path) runner = task.SyncRunner() From b2d06645af336867ec081afbafb029aad22b6518 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Mon, 4 Nov 2019 14:28:06 +0000 Subject: [PATCH 2/8] Address Freso's comment Signed-off-by: JoeLametta --- whipper/image/toc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/whipper/image/toc.py b/whipper/image/toc.py index d3cc3ccf..f9cef34a 100644 --- a/whipper/image/toc.py +++ b/whipper/image/toc.py @@ -140,7 +140,7 @@ def __init__(self, path): """ :type path: str """ - assert isinstance(path, str), "%r is not string" % path + assert isinstance(path, str), "%r is not str" % path self._path = path self._messages = [] self.table = table.Table() From 8446c290e7394fc3ffa39798fdc24c19a21f8955 Mon Sep 17 00:00:00 2001 From: Andreas Oberritter Date: Wed, 30 Oct 2019 20:45:38 +0100 Subject: [PATCH 3/8] accuraterip-checksum.c: Port to Python 3 Accuraterip-checksum extension will be Python 3 only (JoeLametta). Co-authored-by: JoeLametta Signed-off-by: Andreas Oberritter Signed-off-by: JoeLametta --- README.md | 16 +--------------- src/accuraterip-checksum.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 4c5e5993..52dd9284 100644 --- a/README.md +++ b/README.md @@ -27,8 +27,7 @@ We've nearly completed porting the codebase to Python 3 (Python 2 won't be suppo - [Building](#building) 1. [Required dependencies](#required-dependencies) 2. [Fetching the source code](#fetching-the-source-code) - 3. [Building the bundled dependencies](#building-the-bundled-dependencies) - 4. [Finalizing the build](#finalizing-the-build) + 3. [Finalizing the build](#finalizing-the-build) - [Usage](#usage) - [Getting started](#getting-started) - [Configuration file documentation](#configuration-file-documentation) @@ -164,19 +163,6 @@ git clone https://github.com/whipper-team/whipper.git cd whipper ``` -### Building the bundled dependencies - -Whipper uses and packages a slightly different version of the `accuraterip-checksum` tool: - -You can edit the install path in `config.mk` - -```bash -cd src -make -sudo make install -cd .. -``` - ### Finalizing the build Install whipper: `python2 setup.py install` diff --git a/src/accuraterip-checksum.c b/src/accuraterip-checksum.c index ee784d9e..652b3858 100644 --- a/src/accuraterip-checksum.c +++ b/src/accuraterip-checksum.c @@ -147,7 +147,13 @@ static PyMethodDef accuraterip_methods[] = { { NULL, NULL, 0, NULL }, }; -PyMODINIT_FUNC initaccuraterip(void) +static struct PyModuleDef accuraterip_module = { + .m_base = PyModuleDef_HEAD_INIT, + .m_name = "accuraterip", + .m_methods = accuraterip_methods, +}; + +PyMODINIT_FUNC PyInit_accuraterip(void) { - Py_InitModule("accuraterip", accuraterip_methods); + return PyModule_Create(&accuraterip_module); } From 35201d52902dedea60fc2fdd1b69851b5c1326b7 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Sat, 10 Aug 2019 09:10:00 +0000 Subject: [PATCH 4/8] Address errors, improvements, formatting - Removed unused code not portable due to buffer() use - raw_input() does not exist in Python 3 - Fixed octal constant syntax for Python 3 - Fixed TypeError - Replace if not exists: makedirs(path) with single call: using makedirs(path, exist_ok=True) - Class inherits from object, can be safely removed from bases in python3: pylint's useless-object-inheritance (W0235) check Signed-off-by: JoeLametta --- whipper/common/accurip.py | 11 ++++------- whipper/common/common.py | 2 +- whipper/common/directory.py | 11 ++++------- whipper/common/mbngs.py | 4 ++-- whipper/common/path.py | 2 +- whipper/common/program.py | 5 ++--- whipper/common/renamer.py | 4 ++-- whipper/extern/asyncsub.py | 24 ------------------------ whipper/extern/freedb.py | 5 +---- whipper/extern/task/task.py | 4 ++-- whipper/image/cue.py | 6 +++--- whipper/image/image.py | 2 +- whipper/image/table.py | 2 +- whipper/image/toc.py | 2 +- whipper/program/cdparanoia.py | 2 +- whipper/program/cdrdao.py | 3 +-- whipper/result/result.py | 4 ++-- whipper/test/test_result_logger.py | 3 ++- 18 files changed, 31 insertions(+), 65 deletions(-) diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py index 0cd07879..7ddfb41c 100644 --- a/whipper/common/accurip.py +++ b/whipper/common/accurip.py @@ -21,7 +21,6 @@ import requests import struct -from errno import EEXIST from os import makedirs from os.path import dirname, exists, join @@ -40,7 +39,7 @@ class EntryNotFound(Exception): pass -class _AccurateRipResponse(object): +class _AccurateRipResponse: """ An AccurateRip response contains a collection of metadata identifying a particular digital audio compact disc. @@ -143,13 +142,11 @@ def _download_entry(path): def _save_entry(raw_entry, path): logger.debug('saving AccurateRip entry to %s', path) - # XXX: os.makedirs(exist_ok=True) in py3 try: - makedirs(dirname(path)) + makedirs(dirname(path), exist_ok=True) except OSError as e: - if e.errno != EEXIST: - logger.error('could not save entry to %s: %s', path, e) - return + logger.error('could not save entry to %s: %s', path, e) + return open(path, 'wb').write(raw_entry) diff --git a/whipper/common/common.py b/whipper/common/common.py index 3a061762..6c2ead69 100644 --- a/whipper/common/common.py +++ b/whipper/common/common.py @@ -292,7 +292,7 @@ def validate_template(template, kind): 'variable(s): {}'.format(', '.join(matches))) -class VersionGetter(object): +class VersionGetter: """ I get the version of a program by looking for it in command output according to a regexp. diff --git a/whipper/common/directory.py b/whipper/common/directory.py index 910a42e9..55b3880d 100644 --- a/whipper/common/directory.py +++ b/whipper/common/directory.py @@ -19,14 +19,13 @@ # along with whipper. If not, see . from os import getenv, makedirs -from os.path import join, expanduser, exists +from os.path import join, expanduser def config_path(): path = join(getenv('XDG_CONFIG_HOME') or join(expanduser('~'), '.config'), 'whipper') - if not exists(path): - makedirs(path) + makedirs(path, exist_ok=True) return join(path, 'whipper.conf') @@ -35,8 +34,7 @@ def cache_path(name=None): 'whipper') if name: path = join(path, name) - if not exists(path): - makedirs(path) + makedirs(path, exist_ok=True) return path @@ -46,6 +44,5 @@ def data_path(name=None): 'whipper') if name: path = join(path, name) - if not exists(path): - makedirs(path) + makedirs(path, exist_ok=True) return path diff --git a/whipper/common/mbngs.py b/whipper/common/mbngs.py index 50170738..f6b014f8 100644 --- a/whipper/common/mbngs.py +++ b/whipper/common/mbngs.py @@ -45,7 +45,7 @@ def __str__(self): return "Disc not found in MusicBrainz" -class TrackMetadata(object): +class TrackMetadata: artist = None title = None duration = None # in ms @@ -56,7 +56,7 @@ class TrackMetadata(object): mbidWorks = [] -class DiscMetadata(object): +class DiscMetadata: """ :param artist: artist(s) name :param sortName: release artist sort name diff --git a/whipper/common/path.py b/whipper/common/path.py index 7c01d8ff..43c2353c 100644 --- a/whipper/common/path.py +++ b/whipper/common/path.py @@ -21,7 +21,7 @@ import re -class PathFilter(object): +class PathFilter: """ I filter path components for safe storage on file systems. """ diff --git a/whipper/common/program.py b/whipper/common/program.py index 57748624..1d829ae3 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -333,7 +333,7 @@ def getMusicBrainz(self, ittoc, mbdiscid, release=None, country=None, if prompt: guess = (deltas[lowest])[0].mbid - release = raw_input( + release = input( "\nPlease select a release [%s]: " % guess) if not release: @@ -505,8 +505,7 @@ def ripTrack(self, runner, trackResult, offset, device, taglist, stop = self.result.table.getTrackEnd(trackResult.number) dirname = os.path.dirname(trackResult.filename) - if not os.path.exists(dirname): - os.makedirs(dirname) + os.makedirs(dirname, exist_ok=True) if not what: what = 'track %d' % (trackResult.number, ) diff --git a/whipper/common/renamer.py b/whipper/common/renamer.py index 22be22ad..d2af2300 100644 --- a/whipper/common/renamer.py +++ b/whipper/common/renamer.py @@ -24,7 +24,7 @@ """Rename files on file system and inside metafiles in a resumable way.""" -class Operator(object): +class Operator: def __init__(self, statePath, key): self._todo = [] @@ -116,7 +116,7 @@ def addRename(self, source, destination): """ -class Operation(object): +class Operation: def verify(self): """ diff --git a/whipper/extern/asyncsub.py b/whipper/extern/asyncsub.py index a2fe0210..767752bc 100644 --- a/whipper/extern/asyncsub.py +++ b/whipper/extern/asyncsub.py @@ -150,27 +150,3 @@ def recv_some(p, t=.1, e=1, tr=5, stderr=0): else: time.sleep(max((x - time.time()) / tr, 0)) return ''.join(x.decode() for x in y).encode() - - -def send_all(p, data): - while data: - sent = p.send(data) - if sent is None: - raise Exception(message) - data = buffer(data, sent) - - -if __name__ == '__main__': - if sys.platform == 'win32': - shell, commands, tail = ('cmd', ('dir /w', 'echo HELLO WORLD'), '\r\n') - else: - shell, commands, tail = ('sh', ('ls', 'echo HELLO WORLD'), '\n') - - a = Popen(shell, stdin=PIPE, stdout=PIPE) - print(recv_some(a)) - for cmd in commands: - send_all(a, cmd + tail) - print(recv_some(a)) - send_all(a, 'exit' + tail) - print(recv_some(a, e=0)) - a.wait() diff --git a/whipper/extern/freedb.py b/whipper/extern/freedb.py index bd0ee548..4b86de3f 100644 --- a/whipper/extern/freedb.py +++ b/whipper/extern/freedb.py @@ -17,16 +17,13 @@ # USA -import sys - - def digit_sum(i): """returns the sum of all digits for the given integer""" return sum(map(int, str(i))) -class DiscID(object): +class DiscID: def __init__(self, offsets, total_length, track_count, playable_length): """offsets is a list of track offsets, in CD frames total_length is the total length of the disc, in seconds diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index cdd572f9..d739b7b4 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -68,7 +68,7 @@ def _getExceptionMessage(exception, frame=-1, filename=None): % locals() -class LogStub(object): +class LogStub: """ I am a stub for a log interface. """ @@ -243,7 +243,7 @@ def _notifyListeners(self, methodName, *args, **kwargs): # FIXME: should this become a real interface, like in zope ? -class ITaskListener(object): +class ITaskListener: """ I am an interface for objects listening to tasks. """ diff --git a/whipper/image/cue.py b/whipper/image/cue.py index a72e36d4..64b09a7f 100644 --- a/whipper/image/cue.py +++ b/whipper/image/cue.py @@ -58,7 +58,7 @@ """, re.VERBOSE) -class CueFile(object): +class CueFile: """ I represent a .cue file as an object. @@ -138,8 +138,8 @@ def parse(self): seconds = int(m.expand('\\3')) frames = int(m.expand('\\4')) frameOffset = int(frames - + seconds * common.FRAMES_PER_SECOND - + minutes * common.FRAMES_PER_SECOND * 60) + + seconds * common.FRAMES_PER_SECOND + + minutes * common.FRAMES_PER_SECOND * 60) logger.debug('found index %d of track %r in %r:%d', indexNumber, currentTrack, currentFile.path, diff --git a/whipper/image/image.py b/whipper/image/image.py index 8d519839..c68a6f01 100644 --- a/whipper/image/image.py +++ b/whipper/image/image.py @@ -34,7 +34,7 @@ logger = logging.getLogger(__name__) -class Image(object): +class Image: """ :ivar table: The Table of Contents for this image. :vartype table: table.Table diff --git a/whipper/image/table.py b/whipper/image/table.py index d1968352..cbb2711f 100644 --- a/whipper/image/table.py +++ b/whipper/image/table.py @@ -157,7 +157,7 @@ def __repr__(self): self.number, self.absolute, self.path, self.relative, self.counter) -class Table(object): +class Table: """ I represent a table of indexes on a CD. diff --git a/whipper/image/toc.py b/whipper/image/toc.py index f9cef34a..9cf1f129 100644 --- a/whipper/image/toc.py +++ b/whipper/image/toc.py @@ -134,7 +134,7 @@ def getCounterStart(self, counter): return self._sources[-1][1] -class TocFile(object): +class TocFile: def __init__(self, path): """ diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index 5478311d..636e6f5e 100644 --- a/whipper/program/cdparanoia.py +++ b/whipper/program/cdparanoia.py @@ -452,7 +452,7 @@ def __init__(self, path, table, start, stop, overread, offset=0, logger.debug('read and verify with taglist %r', taglist) # FIXME: choose a dir on the same disk/dir as the final path fd, tmppath = tempfile.mkstemp(suffix='.whipper.wav') - os.fchmod(fd, 0644) + os.fchmod(fd, 0o644) os.close(fd) self._tmpwavpath = tmppath diff --git a/whipper/program/cdrdao.py b/whipper/program/cdrdao.py index 365c5ade..9bf33991 100644 --- a/whipper/program/cdrdao.py +++ b/whipper/program/cdrdao.py @@ -151,8 +151,7 @@ def _done(self): t_comp = os.path.abspath(self.toc_path).split(os.sep) t_dirn = os.sep.join(t_comp[:-1]) # If the output path doesn't exist, make it recursively - if not os.path.isdir(t_dirn): - os.makedirs(t_dirn) + os.makedirs(t_dirn, exist_ok=True) t_dst = truncate_filename( os.path.join(t_dirn, t_comp[-1] + '.toc')) shutil.copy(self.tocfile, os.path.join(t_dirn, t_dst)) diff --git a/whipper/result/result.py b/whipper/result/result.py index 7947730f..51a3d6a0 100644 --- a/whipper/result/result.py +++ b/whipper/result/result.py @@ -119,7 +119,7 @@ def getTrackResult(self, number): return None -class Logger(object): +class Logger: """ I log the result of a rip. """ @@ -140,7 +140,7 @@ def log(self, ripResult, epoch=time.time()): # A setuptools-like entry point -class EntryPoint(object): +class EntryPoint: name = 'whipper' @staticmethod diff --git a/whipper/test/test_result_logger.py b/whipper/test/test_result_logger.py index 11fa39bb..6e9d509a 100644 --- a/whipper/test/test_result_logger.py +++ b/whipper/test/test_result_logger.py @@ -163,7 +163,8 @@ def testLogger(self): Dumper=ruamel.yaml.RoundTripDumper ) ) + log_body = "\n".join(actualLines[:-1]).encode() self.assertEqual( parsedLog['SHA-256 hash'], - hashlib.sha256("\n".join(actualLines[:-1])).hexdigest().upper() + hashlib.sha256(log_body).hexdigest().upper() ) From fff3014e159e836b2bb41c48e976bb3beb9ec9ab Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Sat, 10 Aug 2019 09:30:00 +0000 Subject: [PATCH 5/8] Address test failures More details about the fix to the testDuration failure (regression): ``` FAIL: testDuration (whipper.test.test_image_toc.CapitalMergeTestCase) testDuration ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred result = f(*args, **kw) File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed reraise(exc_info[1], exc_info[2]) File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise raise exception.with_traceback(traceback) File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed result = f(*a, **kw) File "/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py", line 271, in testDuration self.assertEqual(self.table.getFrameLength(), 173530) File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual super(_Assertions, self).assertEqual(first, second, msg) File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual assertion_func(first, second, msg=msg) File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual raise self.failureException(msg) twisted.trial.unittest.FailTest: 184930 != 173530 ``` The test fails because if either nextTrack.session or thisTrack.session are None the if is false and the instructions inside it aren't executed. The check for None is needed because Python 3 doesn't allow NoneType comparisons (in Python 2 that was possible). IIRC correctly in that test nextTrack.session has value 2 while thisTrack.session is None. That means the Python 2 version evaluates the if condition to true, while the Python 3 version in the first commit does not. With this change both of the values of nextTrack.session and thisTrack.session are compared as int (if None, the value 1 is used for the comparison - as in disc session 1). Regression introduced in 64dd9d843ab508651863fa30c73ab10a5b1e97f5. Signed-off-by: JoeLametta --- whipper/common/accurip.py | 6 ++++-- whipper/common/cache.py | 2 +- whipper/common/config.py | 2 +- whipper/common/renamer.py | 5 +++-- whipper/image/table.py | 22 ++++++++++++---------- whipper/test/common.py | 2 +- whipper/test/test_command_mblookup.py | 2 +- whipper/test/test_common_accurip.py | 6 +++--- whipper/test/test_common_mbngs.py | 16 ++++++++-------- whipper/test/test_common_renamer.py | 6 +++--- whipper/test/test_image_toc.py | 2 +- 11 files changed, 38 insertions(+), 33 deletions(-) diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py index 7ddfb41c..47b9932d 100644 --- a/whipper/common/accurip.py +++ b/whipper/common/accurip.py @@ -193,7 +193,8 @@ def _match_responses(tracks, responses): for i, track in enumerate(tracks): for v in ('v1', 'v2'): if track.AR[v]['CRC'] == r.checksums[i]: - if r.confidences[i] > track.AR[v]['DBConfidence']: + if (track.AR[v]['DBConfidence'] is None or + r.confidences[i] > track.AR[v]['DBConfidence']): track.AR[v]['DBCRC'] = r.checksums[i] track.AR[v]['DBConfidence'] = r.confidences[i] logger.debug( @@ -242,7 +243,8 @@ def print_report(result): track.AR['v2']['DBCRC'] ) if _f]) max_conf = max( - [track.AR[v]['DBConfidence'] for v in ('v1', 'v2')] + [track.AR[v]['DBConfidence'] for v in ('v1', 'v2') + if track.AR[v]['DBConfidence'] is not None], default=None ) if max_conf: if max_conf < track.AR['DBMaxConfidence']: diff --git a/whipper/common/cache.py b/whipper/common/cache.py index 2f948306..d4133986 100644 --- a/whipper/common/cache.py +++ b/whipper/common/cache.py @@ -98,7 +98,7 @@ def _unpickle(self, default=None): if not os.path.exists(self._path): return - handle = open(self._path) + handle = open(self._path, 'rb') import pickle try: diff --git a/whipper/common/config.py b/whipper/common/config.py index 44a701a2..8774c86c 100644 --- a/whipper/common/config.py +++ b/whipper/common/config.py @@ -44,7 +44,7 @@ def open(self): # Open the file with the correct encoding if os.path.exists(self._path): with codecs.open(self._path, 'r', encoding='utf-8') as f: - self._parser.readfp(f) + self._parser.read_file(f) logger.debug('loaded %d sections from config file', len(self._parser.sections())) diff --git a/whipper/common/renamer.py b/whipper/common/renamer.py index d2af2300..941b0185 100644 --- a/whipper/common/renamer.py +++ b/whipper/common/renamer.py @@ -91,7 +91,7 @@ def start(self): Execute the operations """ - def next(self): + def __next__(self): operation = self._todo[len(self._done)] if self._resuming: operation.redo() @@ -199,7 +199,8 @@ def do(self): (fd, name) = tempfile.mkstemp(suffix='.whipper') for s in handle: - os.write(fd, s.replace(self._source, self._destination)) + os.write(fd, + s.replace(self._source, self._destination).encode()) os.close(fd) os.rename(name, self._path) diff --git a/whipper/image/table.py b/whipper/image/table.py index cbb2711f..131cc470 100644 --- a/whipper/image/table.py +++ b/whipper/image/table.py @@ -220,10 +220,12 @@ def getTrackEnd(self, number): # if on a session border, subtract the session leadin thisTrack = self.tracks[number - 1] nextTrack = self.tracks[number] - if None not in [thisTrack.session, nextTrack.session]: - if nextTrack.session > thisTrack.session: - gap = self._getSessionGap(nextTrack.session) - end -= gap + # The session attribute of a track is None by default (session 1) + # with value > 1 if the track is in another session. Py3 doesn't + # allow NoneType comparisons so we compare against 1 in that case + if int(nextTrack.session or 1) > int(thisTrack.session or 1): + gap = self._getSessionGap(nextTrack.session) + end -= gap return end @@ -286,7 +288,7 @@ def getCDDBValues(self): offset = self.getTrackStart(track.number) + delta offsets.append(offset) debug.append(str(offset)) - seconds = offset / common.FRAMES_PER_SECOND + seconds = offset // common.FRAMES_PER_SECOND n += self._cddbSum(seconds) # the 'real' leadout, not offset by 150 frames @@ -389,11 +391,11 @@ def getMusicBrainzSubmitURL(self): discid = self.getMusicBrainzDiscId() values = self._getMusicBrainzValues() - query = urlencode({ - 'id': discid, - 'toc': ' '.join([str(v) for v in values]), - 'tracks': self.getAudioTracks(), - }) + query = urlencode([ + ('toc', ' '.join([str(v) for v in values])), + ('tracks', self.getAudioTracks()), + ('id', discid), + ]) return urlunparse(( 'https', host, '/cdtoc/attach', '', query, '')) diff --git a/whipper/test/common.py b/whipper/test/common.py index b1ba2e71..a55ee146 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -70,7 +70,7 @@ def readCue(name): version so we can use it in comparisons. """ cuefile = os.path.join(os.path.dirname(__file__), name) - ret = open(cuefile).read().decode('utf-8') + ret = open(cuefile).read() ret = re.sub( 'REM COMMENT "whipper.*', 'REM COMMENT "whipper %s"' % whipper.__version__, diff --git a/whipper/test/test_command_mblookup.py b/whipper/test/test_command_mblookup.py index ee8d6644..c13cb937 100644 --- a/whipper/test/test_command_mblookup.py +++ b/whipper/test/test_command_mblookup.py @@ -16,7 +16,7 @@ def _mock_musicbrainz(discid, country=None, record=False): """Mock function for whipper.common.mbngs.musicbrainz function.""" filename = "whipper.discid.{}.pickle".format(discid) path = os.path.join(os.path.dirname(__file__), filename) - with open(path) as p: + with open(path, "rb") as p: return pickle.load(p) def testMissingReleaseType(self): diff --git a/whipper/test/test_common_accurip.py b/whipper/test/test_common_accurip.py index 9cb1ef2e..4b5378bd 100644 --- a/whipper/test/test_common_accurip.py +++ b/whipper/test/test_common_accurip.py @@ -2,7 +2,7 @@ # vi:si:et:sw=4:sts=4:ts=4 import sys -from StringIO import StringIO +from io import StringIO from os import chmod, makedirs from os.path import dirname, exists, join from shutil import copy, rmtree @@ -22,7 +22,7 @@ class TestAccurateRipResponse(TestCase): def setUpClass(cls): cls.path = 'c/1/2/dBAR-002-0000f21c-00027ef8-05021002.bin' cls.entry = _split_responses( - open(join(dirname(__file__), cls.path[6:])).read() + open(join(dirname(__file__), cls.path[6:]), "rb").read() ) cls.other_path = '4/8/2/dBAR-011-0010e284-009228a3-9809ff0b.bin' @@ -101,7 +101,7 @@ class TestVerifyResult(TestCase): def setUpClass(cls): path = 'c/1/2/dBAR-002-0000f21c-00027ef8-05021002.bin' cls.responses = _split_responses( - open(join(dirname(__file__), path[6:])).read() + open(join(dirname(__file__), path[6:]), "rb").read() ) cls.checksums = { 'v1': ['284fc705', '9cc1f32e'], diff --git a/whipper/test/test_common_mbngs.py b/whipper/test/test_common_mbngs.py index b99a1bcc..492f4669 100644 --- a/whipper/test/test_common_mbngs.py +++ b/whipper/test/test_common_mbngs.py @@ -18,7 +18,7 @@ def testMissingReleaseDate(self): filename = 'whipper.release.c56ff16e-1d81-47de-926f-ba22891bd2bd.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "b.yqPuCBdsV5hrzDvYrw52iK_jE-" @@ -31,7 +31,7 @@ def test2MeterSessies10(self): filename = 'whipper.release.a76714e0-32b1-4ed4-b28e-f86d99642193.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "f7XO36a7n1LCCskkCiulReWbwZA-" @@ -59,7 +59,7 @@ def testBalladOfTheBrokenSeas(self): filename = 'whipper.release.e32ae79a-336e-4d33-945c-8c5e8206dbd3.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "xAq8L4ELMW14.6wI6tt7QAcxiDI-" @@ -93,7 +93,7 @@ def testMalaInCuba(self): filename = 'whipper.release.61c6fd9b-18f8-4a45-963a-ba3c5d990cae.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "u0aKVpO.59JBy6eQRX2vYcoqQZ0-" @@ -130,7 +130,7 @@ def testUnknownArtist(self): filename = 'whipper.release.8478d4da-0cda-4e46-ae8c-1eeacfa5cf37.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "RhrwgVb0hZNkabQCw1dZIhdbMFg-" @@ -167,7 +167,7 @@ def testNenaAndKimWildSingle(self): filename = 'whipper.release.f484a9fc-db21-4106-9408-bcd105c90047.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "X2c2IQ5vUy5x6Jh7Xi_DGHtA1X8-" @@ -214,7 +214,7 @@ def testMissingReleaseGroupType(self): filename = 'whipper.release.d8e6153a-2c47-4804-9d73-0aac1081c3b1.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "xu338_M8WukSRi0J.KTlDoflB8Y-" # disc 4 @@ -228,7 +228,7 @@ def testAllAvailableMetadata(self): filename = 'whipper.release.6109ceed-7e21-490b-b5ad-3a66b4e4cfbb.json' path = os.path.join(os.path.dirname(__file__), filename) handle = open(path, "rb") - response = json.loads(handle.read()) + response = json.loads(handle.read().decode('utf-8')) handle.close() discid = "cHW1Uutl_kyWNaLJsLmTGTe4rnE-" diff --git a/whipper/test/test_common_renamer.py b/whipper/test/test_common_renamer.py index bcfb7dcd..c33ca075 100644 --- a/whipper/test/test_common_renamer.py +++ b/whipper/test/test_common_renamer.py @@ -13,7 +13,7 @@ class RenameInFileTestcase(unittest.TestCase): def setUp(self): (fd, self._path) = tempfile.mkstemp(suffix='.whipper.renamer.infile') - os.write(fd, 'This is a test\nThis is another\n') + os.write(fd, 'This is a test\nThis is another\n'.encode()) os.close(fd) def testVerify(self): @@ -43,7 +43,7 @@ class RenameFileTestcase(unittest.TestCase): def setUp(self): (fd, self._source) = tempfile.mkstemp(suffix='.whipper.renamer.file') - os.write(fd, 'This is a test\nThis is another\n') + os.write(fd, 'This is a test\nThis is another\n'.encode()) os.close(fd) (fd, self._destination) = tempfile.mkstemp( suffix='.whipper.renamer.file') @@ -87,7 +87,7 @@ def setUp(self): (fd, self._source) = tempfile.mkstemp( suffix='.whipper.renamer.operator') - os.write(fd, 'This is a test\nThis is another\n') + os.write(fd, 'This is a test\nThis is another\n'.encode()) os.close(fd) (fd, self._destination) = tempfile.mkstemp( suffix='.whipper.renamer.operator') diff --git a/whipper/test/test_image_toc.py b/whipper/test/test_image_toc.py index 34293d19..04bf04a7 100644 --- a/whipper/test/test_image_toc.py +++ b/whipper/test/test_image_toc.py @@ -364,7 +364,7 @@ def testIndexes(self): ref = self._filterCue( open(os.path.join( os.path.dirname(__file__), - 'strokes-someday.eac.cue')).read()).decode('utf-8') + 'strokes-someday.eac.cue')).read()) common.diffStrings(ref, cue) @staticmethod From bb4c25df978605a7c0fc054f3832c7b52b7df8e6 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Mon, 12 Aug 2019 09:00:00 +0000 Subject: [PATCH 6/8] Address ResourceWarning warnings Signed-off-by: JoeLametta --- whipper/common/accurip.py | 6 ++++-- whipper/common/cache.py | 21 ++++++++++----------- whipper/common/common.py | 10 +++++----- whipper/image/cue.py | 7 +++---- whipper/image/toc.py | 7 +++---- whipper/test/common.py | 3 ++- whipper/test/test_common_accurip.py | 10 ++++------ whipper/test/test_common_renamer.py | 12 ++++++++---- whipper/test/test_image_toc.py | 7 +++---- whipper/test/test_program_soxi.py | 3 ++- whipper/test/test_result_logger.py | 10 +++++----- 11 files changed, 49 insertions(+), 47 deletions(-) diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py index 47b9932d..146b3d84 100644 --- a/whipper/common/accurip.py +++ b/whipper/common/accurip.py @@ -147,7 +147,8 @@ def _save_entry(raw_entry, path): except OSError as e: logger.error('could not save entry to %s: %s', path, e) return - open(path, 'wb').write(raw_entry) + with open(path, 'wb') as f: + f.write(raw_entry) def get_db_entry(path): @@ -160,7 +161,8 @@ def get_db_entry(path): cached_path = join(_CACHE_DIR, path) if exists(cached_path): logger.debug('found accuraterip entry at %s', cached_path) - raw_entry = open(cached_path, 'rb').read() + with open(cached_path, 'rb') as f: + raw_entry = f.read() else: raw_entry = _download_entry(path) if raw_entry: diff --git a/whipper/common/cache.py b/whipper/common/cache.py index d4133986..a3e32a61 100644 --- a/whipper/common/cache.py +++ b/whipper/common/cache.py @@ -98,17 +98,16 @@ def _unpickle(self, default=None): if not os.path.exists(self._path): return - handle = open(self._path, 'rb') - import pickle - - 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: - # can fail for various reasons; in that case, pretend we didn't - # load it - logger.debug(e) + with open(self._path, 'rb') as handle: + import pickle + 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: + # can fail for various reasons; in that case, pretend we didn't + # load it + logger.debug(e) def delete(self): self.object = None diff --git a/whipper/common/common.py b/whipper/common/common.py index 6c2ead69..fd545f03 100644 --- a/whipper/common/common.py +++ b/whipper/common/common.py @@ -317,11 +317,11 @@ def get(self): version = "(Unknown)" try: - p = asyncsub.Popen(self._args, - stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, close_fds=True) - p.wait() - output = asyncsub.recv_some(p, e=0, stderr=1).decode() + with asyncsub.Popen(self._args, + stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, close_fds=True) as p: + p.wait() + output = asyncsub.recv_some(p, e=0, stderr=1).decode() vre = self._regexp.search(output) if vre: version = self._expander % vre.groupdict() diff --git a/whipper/image/cue.py b/whipper/image/cue.py index 64b09a7f..0bba8f4d 100644 --- a/whipper/image/cue.py +++ b/whipper/image/cue.py @@ -25,7 +25,6 @@ """ import re -import codecs from whipper.common import common from whipper.image import table @@ -86,9 +85,9 @@ def parse(self): counter = 0 logger.info('parsing .cue file %r', self._path) - handle = codecs.open(self._path, 'r', 'utf-8') - - for number, line in enumerate(handle.readlines()): + with open(self._path) as f: + content = f.readlines() + for number, line in enumerate(content): line = line.rstrip() m = _REM_RE.search(line) diff --git a/whipper/image/toc.py b/whipper/image/toc.py index 9cf1f129..ed2e484e 100644 --- a/whipper/image/toc.py +++ b/whipper/image/toc.py @@ -25,7 +25,6 @@ """ import re -import codecs from whipper.common import common from whipper.image import table @@ -189,9 +188,9 @@ def parse(self): # the first track's INDEX 1 can only be gotten from the .toc # file once the first pregap is calculated; so we add INDEX 1 # at the end of each parsed TRACK record - handle = codecs.open(self._path, "r", "utf-8") - - for number, line in enumerate(handle.readlines()): + with open(self._path) as f: + content = f.readlines() + for number, line in enumerate(content): line = line.rstrip() # look for CDTEXT stuff in either header or tracks diff --git a/whipper/test/common.py b/whipper/test/common.py index a55ee146..45f83adb 100644 --- a/whipper/test/common.py +++ b/whipper/test/common.py @@ -70,7 +70,8 @@ def readCue(name): version so we can use it in comparisons. """ cuefile = os.path.join(os.path.dirname(__file__), name) - ret = open(cuefile).read() + with open(cuefile) as f: + ret = f.read() ret = re.sub( 'REM COMMENT "whipper.*', 'REM COMMENT "whipper %s"' % whipper.__version__, diff --git a/whipper/test/test_common_accurip.py b/whipper/test/test_common_accurip.py index 4b5378bd..79c6f1c3 100644 --- a/whipper/test/test_common_accurip.py +++ b/whipper/test/test_common_accurip.py @@ -21,9 +21,8 @@ class TestAccurateRipResponse(TestCase): @classmethod def setUpClass(cls): cls.path = 'c/1/2/dBAR-002-0000f21c-00027ef8-05021002.bin' - cls.entry = _split_responses( - open(join(dirname(__file__), cls.path[6:]), "rb").read() - ) + with open(join(dirname(__file__), cls.path[6:]), 'rb') as f: + cls.entry = _split_responses(f.read()) cls.other_path = '4/8/2/dBAR-011-0010e284-009228a3-9809ff0b.bin' def setUp(self): @@ -100,9 +99,8 @@ class TestVerifyResult(TestCase): @classmethod def setUpClass(cls): path = 'c/1/2/dBAR-002-0000f21c-00027ef8-05021002.bin' - cls.responses = _split_responses( - open(join(dirname(__file__), path[6:]), "rb").read() - ) + with open(join(dirname(__file__), path[6:]), 'rb') as f: + cls.responses = _split_responses(f.read()) cls.checksums = { 'v1': ['284fc705', '9cc1f32e'], 'v2': ['dc77f9ab', 'dd97d2c3'], diff --git a/whipper/test/test_common_renamer.py b/whipper/test/test_common_renamer.py index c33ca075..d776e3fd 100644 --- a/whipper/test/test_common_renamer.py +++ b/whipper/test/test_common_renamer.py @@ -25,7 +25,8 @@ def testVerify(self): def testDo(self): o = renamer.RenameInFile(self._path, 'is is a', 'at was some') o.do() - output = open(self._path).read() + with open(self._path) as f: + output = f.read() self.assertEqual(output, 'That was some test\nThat was somenother\n') os.unlink(self._path) @@ -34,7 +35,8 @@ def testSerialize(self): data = o.serialize() o2 = renamer.RenameInFile.deserialize(data) o2.do() - output = open(self._path).read() + with open(self._path) as f: + output = f.read() self.assertEqual(output, 'That was some test\nThat was somenother\n') os.unlink(self._path) @@ -66,7 +68,8 @@ def testVerify(self): def testDo(self): self._operation.do() - output = open(self._destination).read() + with open(self._destination) as f: + output = f.read() self.assertEqual(output, 'This is a test\nThis is another\n') os.unlink(self._destination) @@ -74,7 +77,8 @@ def testSerialize(self): data = self._operation.serialize() o = renamer.RenameFile.deserialize(data) o.do() - output = open(self._destination).read() + with open(self._destination) as f: + output = f.read() self.assertEqual(output, 'This is a test\nThis is another\n') os.unlink(self._destination) diff --git a/whipper/test/test_image_toc.py b/whipper/test/test_image_toc.py index 04bf04a7..79fcfc4a 100644 --- a/whipper/test/test_image_toc.py +++ b/whipper/test/test_image_toc.py @@ -361,10 +361,9 @@ def testIndexes(self): self.assertEqual(i1.path, 'data.wav') cue = self._filterCue(self.toc.table.cue()) - ref = self._filterCue( - open(os.path.join( - os.path.dirname(__file__), - 'strokes-someday.eac.cue')).read()) + with open(os.path.join(os.path.dirname(__file__), + 'strokes-someday.eac.cue')) as f: + ref = self._filterCue(f.read()) common.diffStrings(ref, cue) @staticmethod diff --git a/whipper/test/test_program_soxi.py b/whipper/test/test_program_soxi.py index 32f3e23a..0bd0153c 100644 --- a/whipper/test/test_program_soxi.py +++ b/whipper/test/test_program_soxi.py @@ -27,7 +27,8 @@ class AudioLengthPathTestCase(tcommon.TestCase): def _testSuffix(self, suffix): fd, path = tempfile.mkstemp(suffix=suffix) with os.fdopen(fd, "wb") as temptrack: - temptrack.write(open(base_track_file, "rb").read()) + with open(base_track_file, "rb") as f: + temptrack.write(f.read()) t = AudioLengthTask(path) runner = task.SyncRunner() diff --git a/whipper/test/test_result_logger.py b/whipper/test/test_result_logger.py index 6e9d509a..af8d5200 100644 --- a/whipper/test/test_result_logger.py +++ b/whipper/test/test_result_logger.py @@ -131,20 +131,20 @@ def testLogger(self): logger = WhipperLogger() actual = logger.log(ripResult) actualLines = actual.splitlines() - expectedLines = open( - os.path.join(self.path, 'test_result_logger.log'), 'r' - ).read().splitlines() + with open(os.path.join(self.path, + 'test_result_logger.log'), 'r') as f: + expectedLines = f.read().splitlines() # do not test on version line, date line, or SHA-256 hash line self.assertListEqual(actualLines[2:-1], expectedLines[2:-1]) - self.assertRegexpMatches( + self.assertRegex( actualLines[0], re.compile(( r'Log created by: whipper ' r'[\d]+\.[\d]+\.[\d]+\.dev[\w\.\+]+ \(internal logger\)' )) ) - self.assertRegexpMatches( + self.assertRegex( actualLines[1], re.compile(( r'Log creation date: ' From 42019ce85f1789b7340dd97b2dc324efb7541298 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Wed, 6 Nov 2019 15:17:15 +0000 Subject: [PATCH 7/8] Port (misc) offsets script to Python 3 I've also changed the output format a bit. Signed-off-by: JoeLametta --- misc/offsets.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/misc/offsets.py b/misc/offsets.py index 626f9252..5dac7938 100644 --- a/misc/offsets.py +++ b/misc/offsets.py @@ -6,13 +6,12 @@ import sys -import BeautifulSoup +from bs4 import BeautifulSoup -handle = open(sys.argv[1]) +with open(sys.argv[1]) as f: + doc = f.read() -doc = handle.read() - -soup = BeautifulSoup.BeautifulSoup(doc) +soup = BeautifulSoup(doc) offsets = {} # offset -> total count @@ -50,18 +49,17 @@ # now format it for code inclusion lines = [] -line = 'OFFSETS = "' +line = 'OFFSETS = ("' for offset in offsets: line += offset + ", " if len(line) > 60: - line += "\" + \\" lines.append(line) - line = ' "' + line = ' "' # get last line too, trimming the comma and adding the quote if len(line) > 11: - line = line[:-2] + '"' + line = line[:-2] + '")' lines.append(line) print("\n".join(lines)) From 50c8cbb23786ec588aab14ec98bf534658bd98ad Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Tue, 26 Nov 2019 18:39:02 +0000 Subject: [PATCH 8/8] Update README, .travis.yml and Dockerfile for Python 3 Signed-off-by: JoeLametta --- .travis.yml | 2 +- Dockerfile | 12 ++++++------ README.md | 35 ++++++++++++++++++----------------- setup.py | 2 +- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8d102eba..7f5eae0a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ sudo: required language: python python: - - "2.7" + - "3.5" virtualenv: system_site_packages: false diff --git a/Dockerfile b/Dockerfile index f6ee59c5..b12b9a17 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,11 +1,11 @@ FROM debian:buster RUN apt-get update \ - && apt-get install -y autoconf cdrdao curl eject flac git libiso9660-dev \ - libsndfile1-dev libtool locales make pkgconf python-gobject-2 \ - python-musicbrainzngs python-mutagen python-pip python-requests \ - python-ruamel.yaml python-setuptools sox swig \ - && pip install pycdio==2.1.0 + && apt-get install -y autoconf cdrdao curl eject flac gir1.2-glib-2.0 git libiso9660-dev \ + libsndfile1-dev libtool locales make pkgconf python3-gi \ + python3-musicbrainzngs python3-mutagen python3-pip python3-requests \ + python3-ruamel.yaml python3-setuptools sox swig \ + && pip3 install pycdio==2.1.0 # libcdio-paranoia / libcdio-utils are wrongfully packaged in Debian, thus built manually # see https://github.com/whipper-team/whipper/pull/237#issuecomment-367985625 @@ -44,7 +44,7 @@ RUN echo "LC_ALL=en_US.UTF-8" >> /etc/environment \ # install whipper RUN mkdir /whipper COPY . /whipper/ -RUN cd /whipper && python2 setup.py install \ +RUN cd /whipper && python3 setup.py install \ && rm -rf /whipper \ && whipper -v diff --git a/README.md b/README.md index 52dd9284..ca8afea7 100644 --- a/README.md +++ b/README.md @@ -8,14 +8,12 @@ [![GitHub Issues](https://img.shields.io/github/issues/whipper-team/whipper.svg)](https://github.com/whipper-team/whipper/issues) [![GitHub contributors](https://img.shields.io/github/contributors/whipper-team/whipper.svg)](https://github.com/whipper-team/whipper/graphs/contributors) -Whipper is a Python 2.7 CD-DA ripper based on the [morituri project](https://github.com/thomasvs/morituri) (_CDDA ripper for *nix systems aiming for accuracy over speed_). It started just as a fork of morituri - which development seems to have halted - merging old ignored pull requests, improving it with bugfixes and new features. Nowadays whipper's codebase diverges significantly from morituri's one. +Whipper is a Python 3 (3.5+) CD-DA ripper based on the [morituri project](https://github.com/thomasvs/morituri) (_CDDA ripper for *nix systems aiming for accuracy over speed_). It started just as a fork of morituri - which development seems to have halted - merging old ignored pull requests, improving it with bugfixes and new features. Nowadays whipper's codebase diverges significantly from morituri's one. Whipper is currently developed and tested _only_ on Linux distributions but _may_ work fine on other *nix OSes too. In order to track whipper's latest changes it's advised to check its commit history (README and [CHANGELOG](#changelog) files may not be comprehensive). -We've nearly completed porting the codebase to Python 3 (Python 2 won't be supported anymore in future releases). If you would like to follow the progress of the port e/o help us with it, please check [pull request #411](https://github.com/whipper-team/whipper/pull/411). - ## Table of content - [Rationale](#rationale) @@ -122,33 +120,34 @@ If you are building from a source tarball or checkout, you can choose to use whi Whipper relies on the following packages in order to run correctly and provide all the supported features: -- [cd-paranoia](https://www.gnu.org/software/libcdio/), for the actual ripping +- [cd-paranoia](https://github.com/rocky/libcdio-paranoia), for the actual ripping - To avoid bugs it's advised to use `cd-paranoia` versions ≥ **10.2+0.94+2-2** - The package named `libcdio-utils`, available on Debian and Ubuntu, is affected by a bug (except for Debian testing/sid): it doesn't include the `cd-paranoia` binary (needed by whipper). For more details see: [#888053 (Debian)](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888053), [#889803 (Debian)](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=889803) and [#1750264 (Ubuntu)](https://bugs.launchpad.net/ubuntu/+source/libcdio/+bug/1750264). - [cdrdao](http://cdrdao.sourceforge.net/), for session, TOC, pre-gap, and ISRC extraction - [GObject Introspection](https://wiki.gnome.org/Projects/GObjectIntrospection), to provide GLib-2.0 methods used by `task.py` - [PyGObject](https://pypi.org/project/PyGObject/), required by `task.py` -- [python-musicbrainzngs](https://github.com/alastair/python-musicbrainzngs), for metadata lookup -- [python-mutagen](https://pypi.python.org/pypi/mutagen), for tagging support -- [python-setuptools](https://pypi.python.org/pypi/setuptools), for installation, plugins support -- [python-requests](https://pypi.python.org/pypi/requests), for retrieving AccurateRip database entries +- [musicbrainzngs](https://pypi.org/project/musicbrainzngs/), for metadata lookup +- [mutagen](https://pypi.python.org/pypi/mutagen), for tagging support +- [setuptools](https://pypi.python.org/pypi/setuptools), for installation, plugins support +- [requests](https://pypi.python.org/pypi/requests), for retrieving AccurateRip database entries - [pycdio](https://pypi.python.org/pypi/pycdio/), for drive identification (required for drive offset and caching behavior to be stored in the configuration file). - To avoid bugs it's advised to use the most recent `pycdio` version with the corresponding `libcdio` release or, if stuck to old pycdio versions, **0.20**/**0.21** with `libcdio` ≥ **0.90** ≤ **0.94**. All other combinations won't probably work. - [ruamel.yaml](https://pypi.org/project/ruamel.yaml/), for generating well formed YAML report logfiles - [libsndfile](http://www.mega-nerd.com/libsndfile/), for reading wav files - [flac](https://xiph.org/flac/), for reading flac files - [sox](http://sox.sourceforge.net/), for track peak detection +- [git](https://git-scm.com/) or [mercurial](https://www.mercurial-scm.org/) + - Required either when running whipper without installing it or when building it from its source code (code cloned from a git/mercurial repository). Some dependencies aren't available in the PyPI. They can be probably installed using your distribution's package manager: -- [cd-paranoia](https://www.gnu.org/software/libcdio/) +- [cd-paranoia](https://github.com/rocky/libcdio-paranoia) - [cdrdao](http://cdrdao.sourceforge.net/) - [GObject Introspection](https://wiki.gnome.org/Projects/GObjectIntrospection) - [libsndfile](http://www.mega-nerd.com/libsndfile/) - [flac](https://xiph.org/flac/) - [sox](http://sox.sourceforge.net/) - [git](https://git-scm.com/) or [mercurial](https://www.mercurial-scm.org/) - - Required either when running whipper without installing it or when building it from its source code (code cloned from a git/mercurial repository). PyPI installable dependencies are listed in the [requirements.txt](https://github.com/whipper-team/whipper/blob/master/requirements.txt) file and can be installed issuing the following command: @@ -165,7 +164,7 @@ cd whipper ### Finalizing the build -Install whipper: `python2 setup.py install` +Install whipper: `python3 setup.py install` Note that, depending on the chosen installation path, this command may require elevated rights. @@ -218,7 +217,7 @@ The configuration file is stored in `$XDG_CONFIG_HOME/whipper/whipper.conf`, or See [XDG Base Directory Specification](http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html) -and [ConfigParser](https://docs.python.org/2/library/configparser.html). +and [ConfigParser](https://docs.python.org/3/library/configparser.html). The configuration file consists of newline-delineated `[sections]` containing `key = value` pairs. The sections `[main]` and @@ -257,7 +256,7 @@ To make it easier for developers, you can run whipper straight from the source checkout: ```bash -python2 -m whipper -h +python3 -m whipper -h ``` ## Logger plugins @@ -284,8 +283,10 @@ Whipper searches for logger plugins in the following paths: On a default Debian/Ubuntu installation, the following paths are searched by whipper: - `$HOME/.local/share/whipper/plugins` -- `/usr/local/lib/python2.7/dist-packages/whipper/plugins` -- `/usr/lib/python2.7/dist-packages/whipper/plugins` +- `/usr/local/lib/python3.X/dist-packages/whipper/plugins` +- `/usr/lib/python3.X/dist-packages/whipper/plugins` + +Where `X` stands for the minor version of the Python 3 release available on the system. ### Official logger plugins @@ -322,7 +323,7 @@ Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Make sure you have the latest copy from our [git repository](https://github.com/whipper-team/whipper). Where possible, please include tests for new or changed functionality. You can run tests -with `python -m unittest discover` from your source checkout. +with `python3 -m unittest discover` from your source checkout. ### Developer Certificate of Origin (DCO) @@ -396,7 +397,7 @@ gzip whipper.log And attach the gzipped log file to your bug report. -Without `WHIPPER_LOGFILE` set, logging messages will go to stderr. `WHIPPER_DEBUG` accepts a string of the [default python logging levels](https://docs.python.org/2/library/logging.html#logging-levels). +Without `WHIPPER_LOGFILE` set, logging messages will go to stderr. `WHIPPER_DEBUG` accepts a string of the [default python logging levels](https://docs.python.org/3/library/logging.html#logging-levels). ## Credits diff --git a/setup.py b/setup.py index d107f264..96bebf50 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ maintainer=['The Whipper Team'], url='https://github.com/whipper-team/whipper', license='GPL3', - python_requires='>=2.7,<3', + python_requires='>=3.5', packages=find_packages(), setup_requires=['setuptools_scm'], ext_modules=[