diff --git a/picard/tagger.py b/picard/tagger.py index 77fad8626..78c08d849 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -45,7 +45,6 @@ import argparse from collections import defaultdict from functools import partial -from itertools import chain import logging import os.path import platform @@ -111,11 +110,11 @@ from picard.util import ( decode_filename, encode_filename, is_hidden, + iter_files_from_objects, mbid_validate, normpath, process_events_iter, thread, - uniqify, versions, webbrowser2, ) @@ -582,7 +581,7 @@ class Tagger(QtWidgets.QApplication): def copy_files(self, objects): mimeData = QtCore.QMimeData() - mimeData.setUrls([QtCore.QUrl.fromLocalFile(f.filename) for f in (self.get_files_from_objects(objects))]) + mimeData.setUrls([QtCore.QUrl.fromLocalFile(f.filename) for f in iter_files_from_objects(objects)]) self.clipboard().setMimeData(mimeData) def paste_files(self, target): @@ -645,13 +644,15 @@ class Tagger(QtWidgets.QApplication): item.filename if isinstance(item, File) else '') def get_files_from_objects(self, objects, save=False): - """Return list of files from list of albums, clusters, tracks or files.""" - return uniqify(chain(*[obj.iterfiles(save) for obj in objects])) + """Return list of unique files from list of albums, clusters, tracks or files. + + Note: Consider using picard.util.iter_files_from_objects instead, which returns an iterator. + """ + return list(iter_files_from_objects(objects, save=save)) def save(self, objects): """Save the specified objects.""" - files = self.get_files_from_objects(objects, save=True) - for file in files: + for file in iter_files_from_objects(objects, save=True): file.save() def load_album(self, album_id, discid=None): @@ -707,7 +708,7 @@ class Tagger(QtWidgets.QApplication): if album.id not in self.albums: return album.stop_loading() - self.remove_files(self.get_files_from_objects([album])) + self.remove_files(album.iterfiles()) del self.albums[album.id] if album.release_group: album.release_group.remove_album(album.id) @@ -720,7 +721,7 @@ class Tagger(QtWidgets.QApplication): def remove_nat(self, track): """Remove the specified non-album track.""" log.debug("Removing %r", track) - self.remove_files(self.get_files_from_objects([track])) + self.remove_files(track.iterfiles()) if not self.nats: return self.nats.tracks.remove(track) @@ -801,8 +802,7 @@ class Tagger(QtWidgets.QApplication): """Analyze the file(s).""" if not self.use_acoustid: return - files = self.get_files_from_objects(objs) - for file in files: + for file in iter_files_from_objects(objs): if file.can_analyze(): file.set_pending() self._acoustid.analyze(file, partial(file._lookup_finished, File.LOOKUP_ACOUSTID)) @@ -811,12 +811,11 @@ class Tagger(QtWidgets.QApplication): """Generate the fingerprints without matching the files.""" if not self.use_acoustid: return - files = self.get_files_from_objects(objs) def finished(file, result): file.clear_pending() - for file in files: + for file in iter_files_from_objects(objs): file.set_pending() self._acoustid.fingerprint(file, partial(finished, file)) diff --git a/picard/ui/mainwindow.py b/picard/ui/mainwindow.py index bb0c4c40c..50be1de20 100644 --- a/picard/ui/mainwindow.py +++ b/picard/ui/mainwindow.py @@ -73,6 +73,8 @@ from picard.plugin import ExtensionPoint from picard.track import Track from picard.util import ( icontheme, + iter_files_from_objects, + iter_unique, restore_method, thread, throttle, @@ -1043,16 +1045,16 @@ class MainWindow(QtWidgets.QMainWindow, PreserveGeometry): return QtCore.QUrl.fromLocalFile(url) def play_file(self): - files = self.tagger.get_files_from_objects(self.selected_objects) - for file in files: + for file in iter_files_from_objects(self.selected_objects): QtGui.QDesktopServices.openUrl(self._openUrl(file.filename)) def _on_player_error(self, error, msg): self.set_statusbar_message(msg, echo=log.warning, translate=None) def open_folder(self): - files = self.tagger.get_files_from_objects(self.selected_objects) - folders = set([os.path.dirname(f.filename) for f in files]) + folders = iter_unique( + os.path.dirname(f.filename) for f + in iter_files_from_objects(self.selected_objects)) for folder in folders: QtGui.QDesktopServices.openUrl(self._openUrl(folder)) diff --git a/picard/ui/playertoolbar.py b/picard/ui/playertoolbar.py index 95901a043..ded898627 100644 --- a/picard/ui/playertoolbar.py +++ b/picard/ui/playertoolbar.py @@ -37,6 +37,7 @@ from picard.const.sys import IS_MACOS from picard.util import ( format_time, icontheme, + iter_files_from_objects, ) from picard.ui.widgets import ( @@ -133,7 +134,7 @@ class Player(QtCore.QObject): playlist = QtMultimedia.QMediaPlaylist(self) playlist.setPlaybackMode(QtMultimedia.QMediaPlaylist.Sequential) playlist.addMedia([QtMultimedia.QMediaContent(QtCore.QUrl.fromLocalFile(file.filename)) - for file in self.tagger.get_files_from_objects(self._selected_objects)]) + for file in iter_files_from_objects(self._selected_objects)]) self._player.setPlaylist(playlist) self._player.play() diff --git a/picard/ui/scriptsmenu.py b/picard/ui/scriptsmenu.py index 87786522a..ea768228f 100644 --- a/picard/ui/scriptsmenu.py +++ b/picard/ui/scriptsmenu.py @@ -3,7 +3,7 @@ # Picard, the next-generation MusicBrainz tagger # # Copyright (C) 2018 Laurent Monin -# Copyright (C) 2018 Philipp Wolfer +# Copyright (C) 2018, 2020 Philipp Wolfer # Copyright (C) 2018 Yvan Rivière # # This program is free software; you can redistribute it and/or @@ -36,7 +36,7 @@ from picard.script import ( ScriptParser, ) from picard.track import Track -from picard.util import uniqify +from picard.util import iter_unique class ScriptsMenu(QtWidgets.QMenu): @@ -53,7 +53,7 @@ class ScriptsMenu(QtWidgets.QMenu): s_text = script[3] parser = ScriptParser() - for obj in self._get_unique_metadata_objects(): + for obj in self._iter_unique_metadata_objects(): try: parser.eval(s_text, obj.metadata) obj.update() @@ -66,20 +66,17 @@ class ScriptsMenu(QtWidgets.QMenu): } self.tagger.window.set_statusbar_message(msg, mparms) - def _get_unique_metadata_objects(self): - objs = self._get_metadata_objects(self.tagger.window.selected_objects) - return uniqify(objs) + def _iter_unique_metadata_objects(self): + return iter_unique(self._iter_metadata_objects(self.tagger.window.selected_objects)) - def _get_metadata_objects(self, objs): + def _iter_metadata_objects(self, objs): for obj in objs: if hasattr(obj, 'metadata'): yield obj - if isinstance(obj, Cluster): - yield from self._get_metadata_objects(obj.files) - if isinstance(obj, ClusterList): - yield from self._get_metadata_objects(obj) - if isinstance(obj, Album): - yield from self._get_metadata_objects(obj.tracks) - yield from self._get_metadata_objects(obj.unmatched_files.iterfiles()) - if isinstance(obj, Track): - yield from self._get_metadata_objects(obj.files) + if isinstance(obj, Cluster) or isinstance(obj, Track): + yield from self._iter_metadata_objects(obj.iterfiles()) + elif isinstance(obj, ClusterList): + yield from self._iter_metadata_objects(obj) + elif isinstance(obj, Album): + yield from self._iter_metadata_objects(obj.tracks) + yield from self._iter_metadata_objects(obj.unmatched_files.iterfiles()) diff --git a/picard/ui/searchdialog/album.py b/picard/ui/searchdialog/album.py index b30eca1e4..060f2e5cc 100644 --- a/picard/ui/searchdialog/album.py +++ b/picard/ui/searchdialog/album.py @@ -367,6 +367,6 @@ class AlbumSearchDialog(SearchDialog): release["musicbrainz_albumid"]) album = self.tagger.load_album(release["musicbrainz_albumid"]) if self.cluster: - files = self.tagger.get_files_from_objects([self.cluster]) + files = self.cluster.iterfiles() self.tagger.move_files_to_album(files, release["musicbrainz_albumid"], album) diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 1fc510579..67e178b1f 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -41,6 +41,7 @@ import builtins from collections import namedtuple from collections.abc import Mapping import html +from itertools import chain import json import ntpath from operator import attrgetter @@ -112,6 +113,11 @@ def process_events_iter(iterable, interval=0.1): QtCore.QCoreApplication.processEvents() +def iter_files_from_objects(objects, save=False): + """Creates an iterator over all unique files from list of albums, clusters, tracks or files.""" + return iter_unique(chain(*(obj.iterfiles(save) for obj in objects))) + + _io_encoding = sys.getfilesystemencoding() @@ -362,11 +368,13 @@ def throttle(interval): def uniqify(seq): """Uniqify a list, preserving order""" - # Courtesy of Dave Kirby - # See http://www.peterbe.com/plog/uniqifiers-benchmark + return list(iter_unique(seq)) + + +def iter_unique(seq): + """Creates an iterator only returning unique values from seq""" seen = set() - add_seen = seen.add - return [x for x in seq if x not in seen and not add_seen(x)] + return (x for x in seq if x not in seen and not seen.add(x)) # order is important diff --git a/test/test_utils.py b/test/test_utils.py index fafef4f11..ffece9548 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -4,7 +4,7 @@ # # Copyright (C) 2006-2007 Lukáš Lalinský # Copyright (C) 2010 fatih -# Copyright (C) 2010-2011, 2014, 2018-2019 Philipp Wolfer +# Copyright (C) 2010-2011, 2014, 2018-2020 Philipp Wolfer # Copyright (C) 2012, 2014, 2018 Wieland Hoffmann # Copyright (C) 2013 Ionuț Ciocîrlan # Copyright (C) 2013-2014, 2018-2020 Laurent Monin @@ -30,8 +30,10 @@ import builtins from collections import namedtuple +from collections.abc import Iterator import os.path import unittest +from unittest.mock import Mock from test.picardtestcase import PicardTestCase @@ -42,8 +44,11 @@ from picard.util import ( find_best_match, imageinfo, is_absolute_path, + iter_files_from_objects, + iter_unique, limited_join, sort_by_similarity, + uniqify, ) @@ -450,3 +455,35 @@ class LimitedJoin(PicardTestCase): expected = '0,1,2,3,…,6,7,8,9' result = limited_join(self.list, len(self.list) - 1, ',') self.assertEqual(result, expected) + + +class IterFilesFromObjectsTest(PicardTestCase): + + def test_iterate_only_unique(self): + f1 = Mock() + f2 = Mock() + f3 = Mock() + obj1 = Mock() + obj1.iterfiles = Mock(return_value=[f1, f2]) + obj2 = Mock() + obj2.iterfiles = Mock(return_value=[f2, f3]) + result = iter_files_from_objects([obj1, obj2]) + self.assertTrue(isinstance(result, Iterator)) + self.assertEqual([f1, f2, f3], list(result)) + + +class IterUniqifyTest(PicardTestCase): + + def test_unique(self): + items = [1, 2, 3, 2, 3, 4] + result = uniqify(items) + self.assertEqual([1, 2, 3, 4], result) + + +class IterUniqueTest(PicardTestCase): + + def test_unique(self): + items = [1, 2, 3, 2, 3, 4] + result = iter_unique(items) + self.assertTrue(isinstance(result, Iterator)) + self.assertEqual([1, 2, 3, 4], list(result))