From 282843200808b52523ac3bcd31d8e3beeb819b9c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Apr 2015 00:23:28 +0200 Subject: [PATCH 1/6] core: Deprecate remaining methods that used kwargs --- docs/changelog.rst | 8 ++++++-- mopidy/core/library.py | 4 ---- mopidy/core/playlists.py | 6 +----- mopidy/core/tracklist.py | 21 +++++++++++++-------- mopidy/utils/deprecation.py | 4 ++++ 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 95a3156b..0011d60b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,8 +10,12 @@ v1.1.0 (UNRELEASED) Core API -------- -- Calling :meth:`mopidy.core.library.LibraryController.search`` with ``kwargs`` - as the query is no longer supported (PR: :issue:`1090`) +- Calling the following methods with ``kwargs`` is being deprecated. + (PR: :issue:`1090`) + - :meth:`mopidy.core.library.LibraryController.search`` + - :meth:`mopidy.core.library.PlaylistsController.filter`` + - :meth:`mopidy.core.library.TracklistController.filter`` + - :meth:`mopidy.core.library.TracklistController.remove`` - Updated core controllers to handle backend exceptions in all calls that rely on multiple backends. (Issue: :issue:`667`) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 5971ec6e..7140f2cd 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -263,21 +263,17 @@ class LibraryController(object): # Returns results matching 'a' in any backend search({'any': ['a']}) - search(any=['a']) # Returns results matching artist 'xyz' in any backend search({'artist': ['xyz']}) - search(artist=['xyz']) # Returns results matching 'a' and 'b' and artist 'xyz' in any # backend search({'any': ['a', 'b'], 'artist': ['xyz']}) - search(any=['a', 'b'], artist=['xyz']) # Returns results matching 'a' if within the given URI roots # "file:///media/music" and "spotify:" search({'any': ['a']}, uris=['file:///media/music', 'spotify:']) - search(any=['a'], uris=['file:///media/music', 'spotify:']) :param query: one or more queries to search for :type query: dict diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index b470fa56..9be5efa7 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -156,15 +156,12 @@ class PlaylistsController(object): # Returns track with name 'a' filter({'name': 'a'}) - filter(name='a') # Returns track with URI 'xyz' filter({'uri': 'xyz'}) - filter(uri='xyz') # Returns track with name 'a' and URI 'xyz' filter({'name': 'a', 'uri': 'xyz'}) - filter(name='a', uri='xyz') :param criteria: one or more criteria to match by :type criteria: dict @@ -179,8 +176,7 @@ class PlaylistsController(object): validation.check_query( criteria, validation.PLAYLIST_FIELDS, list_values=False) - # TODO: stop using self playlists - matches = self.playlists + matches = self.playlists # TODO: stop using self playlists for (key, value) in criteria.iteritems(): matches = filter(lambda p: getattr(p, key) == value, matches) return matches diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 21c6d86a..596f759f 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -398,34 +398,34 @@ class TracklistController(object): # Returns tracks with TLIDs 1, 2, 3, or 4 (tracklist ID) filter({'tlid': [1, 2, 3, 4]}) - filter(tlid=[1, 2, 3, 4]) # Returns track with IDs 1, 5, or 7 filter({'id': [1, 5, 7]}) - filter(id=[1, 5, 7]) # Returns track with URIs 'xyz' or 'abc' filter({'uri': ['xyz', 'abc']}) - filter(uri=['xyz', 'abc']) # Returns tracks with ID 1 and URI 'xyz' filter({'id': [1], 'uri': ['xyz']}) - filter(id=[1], uri=['xyz']) # Returns track with a matching ID (1, 3 or 6) and a matching URI # ('xyz' or 'abc') filter({'id': [1, 3, 6], 'uri': ['xyz', 'abc']}) - filter(id=[1, 3, 6], uri=['xyz', 'abc']) :param criteria: on or more criteria to match by :type criteria: dict, of (string, list) pairs :rtype: list of :class:`mopidy.models.TlTrack` + + .. deprecated:: 1.1 + Providing the criteria via ``kwargs`` is no longer supported. """ + if kwargs: + deprecation.warn('core.tracklist.filter:kwargs_criteria') + criteria = criteria or kwargs tlids = criteria.pop('tlid', []) validation.check_query(criteria, validation.TRACKLIST_FIELDS) validation.check_instances(tlids, int) - # TODO: deprecate kwargs # TODO: id=[1, 2, 3] filtering can't possibly be working matches = self._tl_tracks @@ -481,9 +481,14 @@ class TracklistController(object): :param criteria: on or more criteria to match by :type criteria: dict :rtype: list of :class:`mopidy.models.TlTrack` that was removed + + .. deprecated:: 1.1 + Providing the criteria via ``kwargs`` is no longer supported. """ - # TODO: deprecate kwargs - tl_tracks = self.filter(criteria, **kwargs) + if kwargs: + deprecation.warn('core.tracklist.remove:kwargs_criteria') + + tl_tracks = self.filter(criteria or kwargs) for tl_track in tl_tracks: position = self._tl_tracks.index(tl_track) del self._tl_tracks[position] diff --git a/mopidy/utils/deprecation.py b/mopidy/utils/deprecation.py index 4f26b41b..db263e6d 100644 --- a/mopidy/utils/deprecation.py +++ b/mopidy/utils/deprecation.py @@ -40,6 +40,10 @@ _MESSAGES = { 'tracklist.add() "tracks" argument is deprecated', 'core.tracklist.add:uri_arg': 'tracklist.add() "uri" argument is deprecated', + 'core.tracklist.filter:kwargs_criteria': + 'tracklist.filter() with "kwargs" as criteria is deprecated', + 'core.tracklist.remove:kwargs_criteria': + 'tracklist.remove() with "kwargs" as criteria is deprecated', 'models.immutable.copy': 'ImmutableObject.copy() is deprecated, use ImmutableObject.replace()', From efad50c253dbf4c53feb4fe81addcdea8d392e0d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Apr 2015 00:25:01 +0200 Subject: [PATCH 2/6] cleanup: Stop using deprecated copy() --- mopidy/core/playlists.py | 2 +- mopidy/local/commands.py | 2 +- mopidy/m3u/playlists.py | 2 +- mopidy/m3u/translator.py | 6 +-- mopidy/mpd/protocol/music_db.py | 2 +- mopidy/stream/actor.py | 2 +- tests/audio/test_utils.py | 80 ++++++++++++++--------------- tests/core/test_events.py | 2 +- tests/m3u/test_playlists.py | 18 +++---- tests/m3u/test_translator.py | 6 +-- tests/mpd/protocol/test_music_db.py | 2 +- tests/mpd/test_translator.py | 16 +++--- 12 files changed, 70 insertions(+), 70 deletions(-) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 9be5efa7..aa5befaf 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -92,7 +92,7 @@ class PlaylistsController(object): # Use the playlist name from as_list() because it knows about any # playlist folder hierarchy, which lookup() does not. return [ - playlists[r.uri].copy(name=r.name) + playlists[r.uri].replace(name=r.name) for r in playlist_refs if playlists[r.uri] is not None] else: return [ diff --git a/mopidy/local/commands.py b/mopidy/local/commands.py index 486e205f..c8c70216 100644 --- a/mopidy/local/commands.py +++ b/mopidy/local/commands.py @@ -143,7 +143,7 @@ class ScanCommand(commands.Command): uri, MIN_DURATION_MS) else: mtime = file_mtimes.get(os.path.join(media_dir, relpath)) - track = utils.convert_tags_to_track(tags).copy( + track = utils.convert_tags_to_track(tags).replace( uri=uri, length=duration, last_modified=mtime) if library.add_supports_tags_and_duration: library.add(track, tags=tags, duration=duration) diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index eaa3d980..bfb27dc0 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -126,4 +126,4 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): file_handle.write(track.uri + '\n') # assert playlist name matches file name/uri - return playlist.copy(uri=uri, name=name) + return playlist.replace(uri=uri, name=name) diff --git a/mopidy/m3u/translator.py b/mopidy/m3u/translator.py index 4eefce9d..177ab6c3 100644 --- a/mopidy/m3u/translator.py +++ b/mopidy/m3u/translator.py @@ -98,13 +98,13 @@ def parse_m3u(file_path, media_dir=None): continue if urlparse.urlsplit(line).scheme: - tracks.append(track.copy(uri=line)) + tracks.append(track.replace(uri=line)) elif os.path.normpath(line) == os.path.abspath(line): path = path_to_uri(line) - tracks.append(track.copy(uri=path)) + tracks.append(track.replace(uri=path)) elif media_dir is not None: path = path_to_uri(os.path.join(media_dir, line)) - tracks.append(track.copy(uri=path)) + tracks.append(track.replace(uri=path)) track = Track() return tracks diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 541fcd6d..83dab871 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -486,7 +486,7 @@ def searchaddpl(context, *args): if not playlist: playlist = context.core.playlists.create(playlist_name).get() tracks = list(playlist.tracks) + _get_tracks(results) - playlist = playlist.copy(tracks=tracks) + playlist = playlist.replace(tracks=tracks) context.core.playlists.save(playlist) diff --git a/mopidy/stream/actor.py b/mopidy/stream/actor.py index 81e07b6d..4b81f60e 100644 --- a/mopidy/stream/actor.py +++ b/mopidy/stream/actor.py @@ -48,7 +48,7 @@ class StreamLibraryProvider(backend.LibraryProvider): try: result = self._scanner.scan(uri) - track = utils.convert_tags_to_track(result.tags).copy( + track = utils.convert_tags_to_track(result.tags).replace( uri=uri, length=result.duration) except exceptions.ScannerError as e: logger.warning('Problem looking up %s: %s', uri, e) diff --git a/tests/audio/test_utils.py b/tests/audio/test_utils.py index a49ead90..200d7729 100644 --- a/tests/audio/test_utils.py +++ b/tests/audio/test_utils.py @@ -59,7 +59,7 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_no(self): del self.tags['track-number'] - self.check(self.track.copy(track_no=None)) + self.check(self.track.replace(track_no=None)) def test_multiple_track_no(self): self.tags['track-number'].append(9) @@ -67,7 +67,7 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_disc_no(self): del self.tags['album-disc-number'] - self.check(self.track.copy(disc_no=None)) + self.check(self.track.replace(disc_no=None)) def test_multiple_track_disc_no(self): self.tags['album-disc-number'].append(9) @@ -75,15 +75,15 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_name(self): del self.tags['title'] - self.check(self.track.copy(name=None)) + self.check(self.track.replace(name=None)) def test_multiple_track_name(self): self.tags['title'] = ['name1', 'name2'] - self.check(self.track.copy(name='name1; name2')) + self.check(self.track.replace(name='name1; name2')) def test_missing_track_musicbrainz_id(self): del self.tags['musicbrainz-trackid'] - self.check(self.track.copy(musicbrainz_id=None)) + self.check(self.track.replace(musicbrainz_id=None)) def test_multiple_track_musicbrainz_id(self): self.tags['musicbrainz-trackid'].append('id') @@ -91,7 +91,7 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_bitrate(self): del self.tags['bitrate'] - self.check(self.track.copy(bitrate=None)) + self.check(self.track.replace(bitrate=None)) def test_multiple_track_bitrate(self): self.tags['bitrate'].append(1234) @@ -99,15 +99,15 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_genre(self): del self.tags['genre'] - self.check(self.track.copy(genre=None)) + self.check(self.track.replace(genre=None)) def test_multiple_track_genre(self): self.tags['genre'] = ['genre1', 'genre2'] - self.check(self.track.copy(genre='genre1; genre2')) + self.check(self.track.replace(genre='genre1; genre2')) def test_missing_track_date(self): del self.tags['date'] - self.check(self.track.copy(date=None)) + self.check(self.track.replace(date=None)) def test_multiple_track_date(self): self.tags['date'].append(datetime.date(2030, 1, 1)) @@ -115,25 +115,25 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_comment(self): del self.tags['comment'] - self.check(self.track.copy(comment=None)) + self.check(self.track.replace(comment=None)) def test_multiple_track_comment(self): self.tags['comment'] = ['comment1', 'comment2'] - self.check(self.track.copy(comment='comment1; comment2')) + self.check(self.track.replace(comment='comment1; comment2')) def test_missing_track_artist_name(self): del self.tags['artist'] - self.check(self.track.copy(artists=[])) + self.check(self.track.replace(artists=[])) def test_multiple_track_artist_name(self): self.tags['artist'] = ['name1', 'name2'] artists = [Artist(name='name1'), Artist(name='name2')] - self.check(self.track.copy(artists=artists)) + self.check(self.track.replace(artists=artists)) def test_missing_track_artist_musicbrainz_id(self): del self.tags['musicbrainz-artistid'] - artist = list(self.track.artists)[0].copy(musicbrainz_id=None) - self.check(self.track.copy(artists=[artist])) + artist = list(self.track.artists)[0].replace(musicbrainz_id=None) + self.check(self.track.replace(artists=[artist])) def test_multiple_track_artist_musicbrainz_id(self): self.tags['musicbrainz-artistid'].append('id') @@ -141,25 +141,25 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_track_composer_name(self): del self.tags['composer'] - self.check(self.track.copy(composers=[])) + self.check(self.track.replace(composers=[])) def test_multiple_track_composer_name(self): self.tags['composer'] = ['composer1', 'composer2'] composers = [Artist(name='composer1'), Artist(name='composer2')] - self.check(self.track.copy(composers=composers)) + self.check(self.track.replace(composers=composers)) def test_missing_track_performer_name(self): del self.tags['performer'] - self.check(self.track.copy(performers=[])) + self.check(self.track.replace(performers=[])) def test_multiple_track_performe_name(self): self.tags['performer'] = ['performer1', 'performer2'] performers = [Artist(name='performer1'), Artist(name='performer2')] - self.check(self.track.copy(performers=performers)) + self.check(self.track.replace(performers=performers)) def test_missing_album_name(self): del self.tags['album'] - self.check(self.track.copy(album=None)) + self.check(self.track.replace(album=None)) def test_multiple_album_name(self): self.tags['album'].append('album2') @@ -167,9 +167,9 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_album_musicbrainz_id(self): del self.tags['musicbrainz-albumid'] - album = self.track.album.copy(musicbrainz_id=None, - images=[]) - self.check(self.track.copy(album=album)) + album = self.track.album.replace(musicbrainz_id=None, + images=[]) + self.check(self.track.replace(album=album)) def test_multiple_album_musicbrainz_id(self): self.tags['musicbrainz-albumid'].append('id') @@ -177,8 +177,8 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_album_num_tracks(self): del self.tags['track-count'] - album = self.track.album.copy(num_tracks=None) - self.check(self.track.copy(album=album)) + album = self.track.album.replace(num_tracks=None) + self.check(self.track.replace(album=album)) def test_multiple_album_num_tracks(self): self.tags['track-count'].append(9) @@ -186,8 +186,8 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_album_num_discs(self): del self.tags['album-disc-count'] - album = self.track.album.copy(num_discs=None) - self.check(self.track.copy(album=album)) + album = self.track.album.replace(num_discs=None) + self.check(self.track.replace(album=album)) def test_multiple_album_num_discs(self): self.tags['album-disc-count'].append(9) @@ -195,21 +195,21 @@ class TagsToTrackTest(unittest.TestCase): def test_missing_album_artist_name(self): del self.tags['album-artist'] - album = self.track.album.copy(artists=[]) - self.check(self.track.copy(album=album)) + album = self.track.album.replace(artists=[]) + self.check(self.track.replace(album=album)) def test_multiple_album_artist_name(self): self.tags['album-artist'] = ['name1', 'name2'] artists = [Artist(name='name1'), Artist(name='name2')] - album = self.track.album.copy(artists=artists) - self.check(self.track.copy(album=album)) + album = self.track.album.replace(artists=artists) + self.check(self.track.replace(album=album)) def test_missing_album_artist_musicbrainz_id(self): del self.tags['musicbrainz-albumartistid'] albumartist = list(self.track.album.artists)[0] - albumartist = albumartist.copy(musicbrainz_id=None) - album = self.track.album.copy(artists=[albumartist]) - self.check(self.track.copy(album=album)) + albumartist = albumartist.replace(musicbrainz_id=None) + album = self.track.album.replace(artists=[albumartist]) + self.check(self.track.replace(album=album)) def test_multiple_album_artist_musicbrainz_id(self): self.tags['musicbrainz-albumartistid'].append('id') @@ -218,30 +218,30 @@ class TagsToTrackTest(unittest.TestCase): def test_stream_organization_track_name(self): del self.tags['title'] self.tags['organization'] = ['organization'] - self.check(self.track.copy(name='organization')) + self.check(self.track.replace(name='organization')) def test_multiple_organization_track_name(self): del self.tags['title'] self.tags['organization'] = ['organization1', 'organization2'] - self.check(self.track.copy(name='organization1; organization2')) + self.check(self.track.replace(name='organization1; organization2')) # TODO: combine all comment types? def test_stream_location_track_comment(self): del self.tags['comment'] self.tags['location'] = ['location'] - self.check(self.track.copy(comment='location')) + self.check(self.track.replace(comment='location')) def test_multiple_location_track_comment(self): del self.tags['comment'] self.tags['location'] = ['location1', 'location2'] - self.check(self.track.copy(comment='location1; location2')) + self.check(self.track.replace(comment='location1; location2')) def test_stream_copyright_track_comment(self): del self.tags['comment'] self.tags['copyright'] = ['copyright'] - self.check(self.track.copy(comment='copyright')) + self.check(self.track.replace(comment='copyright')) def test_multiple_copyright_track_comment(self): del self.tags['comment'] self.tags['copyright'] = ['copyright1', 'copyright2'] - self.check(self.track.copy(comment='copyright1; copyright2')) + self.check(self.track.replace(comment='copyright1; copyright2')) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index e916b670..67dc91f0 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -111,7 +111,7 @@ class BackendEventsTest(unittest.TestCase): def test_playlists_save_sends_playlist_changed_event(self, send): playlist = self.core.playlists.create('foo').get() - playlist = playlist.copy(name='bar') + playlist = playlist.replace(name='bar') send.reset_mock() self.core.playlists.save(playlist).get() diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index a294e6cf..5b5eaa33 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -70,7 +70,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertTrue(os.path.exists(path1)) self.assertFalse(os.path.exists(path2)) - playlist = self.core.playlists.save(playlist.copy(name='test2')) + playlist = self.core.playlists.save(playlist.replace(name='test2')) self.assertEqual('test2', playlist.name) self.assertEqual(uri2, playlist.uri) self.assertFalse(os.path.exists(path1)) @@ -93,7 +93,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_playlist_contents_is_written_to_disk(self): track = Track(uri=generate_song(1)) playlist = self.core.playlists.create('test') - playlist = self.core.playlists.save(playlist.copy(tracks=[track])) + playlist = self.core.playlists.save(playlist.replace(tracks=[track])) path = playlist_uri_to_path(playlist.uri, self.playlists_dir) with open(path) as f: @@ -104,7 +104,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_extended_playlist_contents_is_written_to_disk(self): track = Track(uri=generate_song(1), name='Test', length=60000) playlist = self.core.playlists.create('test') - playlist = self.core.playlists.save(playlist.copy(tracks=[track])) + playlist = self.core.playlists.save(playlist.replace(tracks=[track])) path = playlist_uri_to_path(playlist.uri, self.playlists_dir) with open(path) as f: @@ -115,7 +115,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_playlists_are_loaded_at_startup(self): track = Track(uri='dummy:track:path2') playlist = self.core.playlists.create('test') - playlist = playlist.copy(tracks=[track]) + playlist = playlist.replace(tracks=[track]) playlist = self.core.playlists.save(playlist) self.assertEqual(len(self.core.playlists.as_list()), 1) @@ -191,7 +191,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): playlist1 = self.core.playlists.create('test1') self.assertEqual(playlist1, self.core.playlists.lookup(playlist1.uri)) - playlist2 = playlist1.copy(name='test2') + playlist2 = playlist1.replace(name='test2') playlist2 = self.core.playlists.save(playlist2) self.assertIsNone(self.core.playlists.lookup(playlist1.uri)) self.assertEqual(playlist2, self.core.playlists.lookup(playlist2.uri)) @@ -199,7 +199,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_create_replaces_existing_playlist_with_updated_playlist(self): track = Track(uri=generate_song(1)) playlist1 = self.core.playlists.create('test') - playlist1 = self.core.playlists.save(playlist1.copy(tracks=[track])) + playlist1 = self.core.playlists.save(playlist1.replace(tracks=[track])) self.assertEqual(playlist1, self.core.playlists.lookup(playlist1.uri)) playlist2 = self.core.playlists.create('test') @@ -220,7 +220,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_playlist_with_unknown_track(self): track = Track(uri='file:///dev/null') playlist = self.core.playlists.create('test') - playlist = playlist.copy(tracks=[track]) + playlist = playlist.replace(tracks=[track]) playlist = self.core.playlists.save(playlist) self.assertEqual(len(self.core.playlists.as_list()), 1) @@ -244,7 +244,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): check_order(self.core.playlists.as_list(), ['a', 'b', 'c']) playlist = self.core.playlists.lookup('m3u:a.m3u') - playlist = playlist.copy(name='d') + playlist = playlist.replace(name='d') playlist = self.core.playlists.save(playlist) check_order(self.core.playlists.as_list(), ['b', 'c', 'd']) @@ -256,7 +256,7 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_get_items_returns_item_refs(self): track = Track(uri='dummy:a', name='A', length=60000) playlist = self.core.playlists.create('test') - playlist = self.core.playlists.save(playlist.copy(tracks=[track])) + playlist = self.core.playlists.save(playlist.replace(tracks=[track])) item_refs = self.core.playlists.get_items(playlist.uri) diff --git a/tests/m3u/test_translator.py b/tests/m3u/test_translator.py index c84f12bf..32eb9f3b 100644 --- a/tests/m3u/test_translator.py +++ b/tests/m3u/test_translator.py @@ -22,9 +22,9 @@ encoded_uri = path.path_to_uri(encoded_path) song1_track = Track(uri=song1_uri) song2_track = Track(uri=song2_uri) encoded_track = Track(uri=encoded_uri) -song1_ext_track = song1_track.copy(name='song1') -song2_ext_track = song2_track.copy(name='song2', length=60000) -encoded_ext_track = encoded_track.copy(name='æøå') +song1_ext_track = song1_track.replace(name='song1') +song2_ext_track = song2_track.replace(name='song2', length=60000) +encoded_ext_track = encoded_track.replace(name='æøå') # FIXME use mock instead of tempfile.NamedTemporaryFile diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index ca043d3c..73c3b300 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -103,7 +103,7 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): def test_searchaddpl_appends_to_existing_playlist(self): playlist = self.core.playlists.create('my favs').get() - playlist = playlist.copy(tracks=[ + playlist = playlist.replace(tracks=[ Track(uri='dummy:x', name='X'), Track(uri='dummy:y', name='y'), ]) diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index 4e1baf0e..055932fc 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -80,26 +80,26 @@ class TrackMpdFormatTest(unittest.TestCase): self.assertEqual(len(result), 14) def test_track_to_mpd_format_musicbrainz_trackid(self): - track = self.track.copy(musicbrainz_id='foo') + track = self.track.replace(musicbrainz_id='foo') result = translator.track_to_mpd_format(track) self.assertIn(('MUSICBRAINZ_TRACKID', 'foo'), result) def test_track_to_mpd_format_musicbrainz_albumid(self): - album = self.track.album.copy(musicbrainz_id='foo') - track = self.track.copy(album=album) + album = self.track.album.replace(musicbrainz_id='foo') + track = self.track.replace(album=album) result = translator.track_to_mpd_format(track) self.assertIn(('MUSICBRAINZ_ALBUMID', 'foo'), result) def test_track_to_mpd_format_musicbrainz_albumartistid(self): - artist = list(self.track.artists)[0].copy(musicbrainz_id='foo') - album = self.track.album.copy(artists=[artist]) - track = self.track.copy(album=album) + artist = list(self.track.artists)[0].replace(musicbrainz_id='foo') + album = self.track.album.replace(artists=[artist]) + track = self.track.replace(album=album) result = translator.track_to_mpd_format(track) self.assertIn(('MUSICBRAINZ_ALBUMARTISTID', 'foo'), result) def test_track_to_mpd_format_musicbrainz_artistid(self): - artist = list(self.track.artists)[0].copy(musicbrainz_id='foo') - track = self.track.copy(artists=[artist]) + artist = list(self.track.artists)[0].replace(musicbrainz_id='foo') + track = self.track.replace(artists=[artist]) result = translator.track_to_mpd_format(track) self.assertIn(('MUSICBRAINZ_ARTISTID', 'foo'), result) From d2f9733296a313ae38917fd2d01fc2ff6181c02b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Apr 2015 00:28:09 +0200 Subject: [PATCH 3/6] core: Track.id was removed five years ago, update docs. --- mopidy/core/tracklist.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 596f759f..9d4f960b 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -399,25 +399,19 @@ class TracklistController(object): # Returns tracks with TLIDs 1, 2, 3, or 4 (tracklist ID) filter({'tlid': [1, 2, 3, 4]}) - # Returns track with IDs 1, 5, or 7 - filter({'id': [1, 5, 7]}) - # Returns track with URIs 'xyz' or 'abc' filter({'uri': ['xyz', 'abc']}) - # Returns tracks with ID 1 and URI 'xyz' - filter({'id': [1], 'uri': ['xyz']}) - - # Returns track with a matching ID (1, 3 or 6) and a matching URI - # ('xyz' or 'abc') - filter({'id': [1, 3, 6], 'uri': ['xyz', 'abc']}) + # Returns track with a matching TLIDs (1, 3 or 6) and a + # matching URI ('xyz' or 'abc') + filter({'tlid': [1, 3, 6], 'uri': ['xyz', 'abc']}) :param criteria: on or more criteria to match by :type criteria: dict, of (string, list) pairs :rtype: list of :class:`mopidy.models.TlTrack` .. deprecated:: 1.1 - Providing the criteria via ``kwargs`` is no longer supported. + Providing the criteria via ``kwargs``. """ if kwargs: deprecation.warn('core.tracklist.filter:kwargs_criteria') @@ -426,7 +420,6 @@ class TracklistController(object): tlids = criteria.pop('tlid', []) validation.check_query(criteria, validation.TRACKLIST_FIELDS) validation.check_instances(tlids, int) - # TODO: id=[1, 2, 3] filtering can't possibly be working matches = self._tl_tracks for (key, values) in criteria.items(): From 7459e9c9d847887681ccf6306a632175a4d22ab2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Apr 2015 21:11:06 +0200 Subject: [PATCH 4/6] mpd: Stop using deprecated kwarg based calls --- mopidy/mpd/protocol/current_playlist.py | 14 +++++++------- mopidy/mpd/protocol/music_db.py | 7 +++++-- mopidy/mpd/protocol/playback.py | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index ea815c6a..e406f2ca 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -90,7 +90,7 @@ def delete(context, position): if not tl_tracks: raise exceptions.MpdArgError('Bad song index', command='delete') for (tlid, _) in tl_tracks: - context.core.tracklist.remove(tlid=[tlid]) + context.core.tracklist.remove({'tlid': [tlid]}) @protocol.commands.add('deleteid', tlid=protocol.UINT) @@ -102,7 +102,7 @@ def deleteid(context, tlid): Deletes the song ``SONGID`` from the playlist """ - tl_tracks = context.core.tracklist.remove(tlid=[tlid]).get() + tl_tracks = context.core.tracklist.remove({'tlid': [tlid]}).get() if not tl_tracks: raise exceptions.MpdNoExistError('No such song') @@ -147,7 +147,7 @@ def moveid(context, tlid, to): the playlist. If ``TO`` is negative, it is relative to the current song in the playlist (if there is one). """ - tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get() + tl_tracks = context.core.tracklist.filter({'tlid': [tlid]}).get() if not tl_tracks: raise exceptions.MpdNoExistError('No such song') position = context.core.tracklist.index(tl_tracks[0]).get() @@ -185,7 +185,7 @@ def playlistfind(context, tag, needle): - does not add quotes around the tag. """ if tag == 'filename': - tl_tracks = context.core.tracklist.filter(uri=[needle]).get() + tl_tracks = context.core.tracklist.filter({'uri': [needle]}).get() if not tl_tracks: return None position = context.core.tracklist.index(tl_tracks[0]).get() @@ -204,7 +204,7 @@ def playlistid(context, tlid=None): and specifies a single song to display info for. """ if tlid is not None: - tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get() + tl_tracks = context.core.tracklist.filter({'tlid': [tlid]}).get() if not tl_tracks: raise exceptions.MpdNoExistError('No such song') position = context.core.tracklist.index(tl_tracks[0]).get() @@ -370,8 +370,8 @@ def swapid(context, tlid1, tlid2): Swaps the positions of ``SONG1`` and ``SONG2`` (both song ids). """ - tl_tracks1 = context.core.tracklist.filter(tlid=[tlid1]).get() - tl_tracks2 = context.core.tracklist.filter(tlid=[tlid2]).get() + tl_tracks1 = context.core.tracklist.filter({'tlid': [tlid1]}).get() + tl_tracks2 = context.core.tracklist.filter({'tlid': [tlid2]}).get() if not tl_tracks1 or not tl_tracks2: raise exceptions.MpdNoExistError('No such song') position1 = context.core.tracklist.index(tl_tracks1[0]).get() diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 83dab871..0e59706f 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -6,6 +6,7 @@ import warnings from mopidy.models import Track from mopidy.mpd import exceptions, protocol, translator +from mopidy.utils import deprecation _SEARCH_MAPPING = { 'album': 'album', @@ -142,7 +143,8 @@ def find(context, *args): except ValueError: return - results = context.core.library.search(query=query, exact=True).get() + with deprecation.ignore('core.library.search:empty_query'): + results = context.core.library.search(query=query, exact=True).get() result_tracks = [] if ('artist' not in query and 'albumartist' not in query and @@ -422,7 +424,8 @@ def search(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: return - results = context.core.library.search(query).get() + with deprecation.ignore('core.library.search:empty_query'): + results = context.core.library.search(query).get() artists = [_artist_as_track(a) for a in _get_artists(results)] albums = [_album_as_track(a) for a in _get_albums(results)] tracks = _get_tracks(results) diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index 6beb4277..a537819c 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -217,7 +217,7 @@ def playid(context, tlid): """ if tlid == -1: return _play_minus_one(context) - tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get() + tl_tracks = context.core.tracklist.filter({'tlid': [tlid]}).get() if not tl_tracks: raise exceptions.MpdNoExistError('No such song') return context.core.playback.play(tl_tracks[0]).get() From ab761c45964fd8b1ccdf7c4c9b8ca9db1d3a37e7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Apr 2015 21:11:59 +0200 Subject: [PATCH 5/6] core: Stop using kwarg based remove call --- mopidy/core/tracklist.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 9d4f960b..e0497a9a 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -548,7 +548,7 @@ class TracklistController(object): def _mark_played(self, tl_track): """Internal method for :class:`mopidy.core.PlaybackController`.""" if self.consume and tl_track is not None: - self.remove(tlid=[tl_track.tlid]) + self.remove({'tlid': [tl_track.tlid]}) return True return False From 81fd426caf980fa1a0b4315ba90b529e6909ccbd Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Apr 2015 21:12:18 +0200 Subject: [PATCH 6/6] tests: Update tests to not used deprecated kwargs --- tests/core/test_events.py | 2 +- tests/core/test_playlists.py | 2 +- tests/core/test_tracklist.py | 8 +++---- tests/local/test_tracklist.py | 41 ++++++++++++++++++----------------- tests/m3u/test_playlists.py | 1 + 5 files changed, 28 insertions(+), 26 deletions(-) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index 67dc91f0..157acffd 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -71,7 +71,7 @@ class BackendEventsTest(unittest.TestCase): self.core.tracklist.add(uris=['dummy:a']).get() send.reset_mock() - self.core.tracklist.remove(uri=['dummy:a']).get() + self.core.tracklist.remove({'uri': ['dummy:a']}).get() self.assertEqual(send.call_args[0][0], 'tracklist_changed') diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index 1ccc1815..febff62b 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -240,7 +240,7 @@ class DeprecatedFilterPlaylistsTest(BasePlaylistsTest): return super(DeprecatedFilterPlaylistsTest, self).run(result) def test_filter_returns_matching_playlists(self): - result = self.core.playlists.filter(name='A') + result = self.core.playlists.filter({'name': 'A'}) self.assertEqual(2, len(result)) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 24a9ef0f..1ff089cb 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -64,7 +64,7 @@ class TracklistTest(unittest.TestCase): tl_tracks, self.core.tracklist.tl_tracks[-len(tl_tracks):]) def test_remove_removes_tl_tracks_matching_query(self): - tl_tracks = self.core.tracklist.remove(name=['foo']) + tl_tracks = self.core.tracklist.remove({'name': ['foo']}) self.assertEqual(2, len(tl_tracks)) self.assertListEqual(self.tl_tracks[:2], tl_tracks) @@ -82,7 +82,7 @@ class TracklistTest(unittest.TestCase): self.assertListEqual(self.tl_tracks[2:], self.core.tracklist.tl_tracks) def test_filter_returns_tl_tracks_matching_query(self): - tl_tracks = self.core.tracklist.filter(name=['foo']) + tl_tracks = self.core.tracklist.filter({'name': ['foo']}) self.assertEqual(2, len(tl_tracks)) self.assertListEqual(self.tl_tracks[:2], tl_tracks) @@ -95,10 +95,10 @@ class TracklistTest(unittest.TestCase): def test_filter_fails_if_values_isnt_iterable(self): with self.assertRaises(ValueError): - self.core.tracklist.filter(tlid=3) + self.core.tracklist.filter({'tlid': 3}) def test_filter_fails_if_values_is_a_string(self): with self.assertRaises(ValueError): - self.core.tracklist.filter(uri='a') + self.core.tracklist.filter({'uri': 'a'}) # TODO Extract tracklist tests from the local backend tests diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index ca36ac44..f405f218 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -77,53 +77,54 @@ class LocalTracklistProviderTest(unittest.TestCase): def test_filter_by_tlid(self): tl_track = self.controller.tl_tracks[1] self.assertEqual( - [tl_track], self.controller.filter(tlid=[tl_track.tlid])) + [tl_track], self.controller.filter({'tlid': [tl_track.tlid]})) @populate_tracklist def test_filter_by_uri(self): tl_track = self.controller.tl_tracks[1] self.assertEqual( - [tl_track], self.controller.filter(uri=[tl_track.track.uri])) + [tl_track], self.controller.filter({'uri': [tl_track.track.uri]})) @populate_tracklist def test_filter_by_uri_returns_nothing_for_invalid_uri(self): - self.assertEqual([], self.controller.filter(uri=['foobar'])) + self.assertEqual([], self.controller.filter({'uri': ['foobar']})) def test_filter_by_uri_returns_single_match(self): - track = Track(uri='a') - self.controller.add([Track(uri='z'), track, Track(uri='y')]) - self.assertEqual(track, self.controller.filter(uri=['a'])[0].track) + t = Track(uri='a') + self.controller.add([Track(uri='z'), t, Track(uri='y')]) + self.assertEqual(t, self.controller.filter({'uri': ['a']})[0].track) def test_filter_by_uri_returns_multiple_matches(self): track = Track(uri='a') self.controller.add([Track(uri='z'), track, track]) - tl_tracks = self.controller.filter(uri=['a']) + tl_tracks = self.controller.filter({'uri': ['a']}) self.assertEqual(track, tl_tracks[0].track) self.assertEqual(track, tl_tracks[1].track) def test_filter_by_uri_returns_nothing_if_no_match(self): self.controller.playlist = Playlist( tracks=[Track(uri='z'), Track(uri='y')]) - self.assertEqual([], self.controller.filter(uri=['a'])) + self.assertEqual([], self.controller.filter({'uri': ['a']})) def test_filter_by_multiple_criteria_returns_elements_matching_all(self): - track1 = Track(uri='a', name='x') - track2 = Track(uri='b', name='x') - track3 = Track(uri='b', name='y') - self.controller.add([track1, track2, track3]) + t1 = Track(uri='a', name='x') + t2 = Track(uri='b', name='x') + t3 = Track(uri='b', name='y') + self.controller.add([t1, t2, t3]) self.assertEqual( - track1, self.controller.filter(uri=['a'], name=['x'])[0].track) + t1, self.controller.filter({'uri': ['a'], 'name': ['x']})[0].track) self.assertEqual( - track2, self.controller.filter(uri=['b'], name=['x'])[0].track) + t2, self.controller.filter({'uri': ['b'], 'name': ['x']})[0].track) self.assertEqual( - track3, self.controller.filter(uri=['b'], name=['y'])[0].track) + t3, self.controller.filter({'uri': ['b'], 'name': ['y']})[0].track) def test_filter_by_criteria_that_is_not_present_in_all_elements(self): track1 = Track() track2 = Track(uri='b') track3 = Track() self.controller.add([track1, track2, track3]) - self.assertEqual(track2, self.controller.filter(uri=['b'])[0].track) + self.assertEqual( + track2, self.controller.filter({'uri': ['b']})[0].track) @populate_tracklist def test_clear(self): @@ -233,17 +234,17 @@ class LocalTracklistProviderTest(unittest.TestCase): track1 = self.controller.tracks[1] track2 = self.controller.tracks[2] version = self.controller.version - self.controller.remove(uri=[track1.uri]) + self.controller.remove({'uri': [track1.uri]}) self.assertLess(version, self.controller.version) self.assertNotIn(track1, self.controller.tracks) self.assertEqual(track2, self.controller.tracks[1]) @populate_tracklist def test_removing_track_that_does_not_exist_does_nothing(self): - self.controller.remove(uri=['/nonexistant']) + self.controller.remove({'uri': ['/nonexistant']}) def test_removing_from_empty_playlist_does_nothing(self): - self.controller.remove(uri=['/nonexistant']) + self.controller.remove({'uri': ['/nonexistant']}) @populate_tracklist def test_remove_lists(self): @@ -251,7 +252,7 @@ class LocalTracklistProviderTest(unittest.TestCase): track1 = self.controller.tracks[1] track2 = self.controller.tracks[2] version = self.controller.version - self.controller.remove(uri=[track0.uri, track2.uri]) + self.controller.remove({'uri': [track0.uri, track2.uri]}) self.assertLess(version, self.controller.version) self.assertNotIn(track0, self.controller.tracks) self.assertNotIn(track2, self.controller.tracks) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index 5b5eaa33..a8caf8fd 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -275,6 +275,7 @@ class DeprecatedM3UPlaylistsProviderTest(M3UPlaylistsProviderTest): def run(self, result=None): with deprecation.ignore(ids=['core.playlists.filter', + 'core.playlists.filter:kwargs_criteria', 'core.playlists.get_playlists']): return super(DeprecatedM3UPlaylistsProviderTest, self).run(result)