From 779af598db860db512677d91eafdfc74bffde2ca Mon Sep 17 00:00:00 2001 From: hugovk Date: Wed, 5 Mar 2014 10:29:16 +0200 Subject: [PATCH] Refactor to include limit parameter to reduce bandwidth\n\nRefactor calls to chart.getTopArtists, chart.getTopTracks, tag.getTopTags and user.getTopTags to include the limit parameter (where available) to reduce the size of data sent by Last.fm.\n\nFor example, getting limit=1 can reduce receiving 101 items to 1, making the test take 0.5s rather than 1.2s.\n\nAlso return a list of TopItems rather than just items, and add cacheable parameter. --- pylast.py | 61 +++++++++++++++++++++++++++----------------------- test_pylast.py | 52 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 81 insertions(+), 32 deletions(-) diff --git a/pylast.py b/pylast.py index 57ab220..f5c73da 100644 --- a/pylast.py +++ b/pylast.py @@ -334,51 +334,55 @@ class _Network(object): return Playlist(user, e_id, self) - def get_top_artists(self, limit=None): - """Returns a sequence of the most played artists.""" + def get_top_artists(self, limit=None, cacheable=True): + """Returns the most played artists as a sequence of TopItem objects.""" + + params = {} + if limit: params["limit"] = limit + + doc = _Request(self, "chart.getTopArtists", params).execute(cacheable) - doc = _Request(self, "chart.getTopArtists").execute(True) seq = [] for node in doc.getElementsByTagName("artist"): - title = _extract(node, "name") - artist = Artist(title, self) - seq.append(artist) - - if limit: - seq = seq[:limit] + artist = Artist(_extract(node, "name"), self) + weight = _number(_extract(node, "playcount")) + seq.append(TopItem(artist, weight)) return seq - def get_top_tracks(self, limit=None): - """Returns a sequence of the most played tracks.""" + def get_top_tracks(self, limit=None, cacheable=True): + """Returns the most played tracks as a sequence of TopItem objects.""" + + params = {} + if limit: params["limit"] = limit + + doc = _Request(self, "chart.getTopTracks", params).execute(cacheable) - doc = _Request(self, "chart.getTopTracks").execute(True) seq = [] for node in doc.getElementsByTagName("track"): title = _extract(node, "name") artist = _extract(node, "name", 1) track = Track(artist, title, self) - seq.append(track) - - if limit: - seq = seq[:limit] + weight = _number(_extract(node, "playcount")) + seq.append(TopItem(track, weight)) return seq - def get_top_tags(self, limit=None): - """Returns a sequence of the most used tags as a sequence of TopItem objects.""" + def get_top_tags(self, limit=None, cacheable=True): + """Returns the most used tags as a sequence of TopItem objects.""" + + # Last.fm has no "limit" parameter for tag.getTopTags + # so we need to get all (250) and then limit locally + doc = _Request(self, "tag.getTopTags").execute(cacheable) - doc = _Request(self, "tag.getTopTags").execute(True) seq = [] for node in doc.getElementsByTagName("tag"): + if len(seq) >= limit: + break tag = Tag(_extract(node, "name"), self) weight = _number(_extract(node, "count")) - seq.append(TopItem(tag, weight)) - if limit: - seq = seq[:limit] - return seq def get_geo_events(self, long=None, lat=None, location=None, distance=None, tag=None, festivalsonly=None, limit=None, cacheable=True): @@ -3510,20 +3514,21 @@ class User(_BaseObject): return seq - def get_top_tags(self, limit=None): + def get_top_tags(self, limit=None, cacheable=True): """Returns a sequence of the top tags used by this user with their counts as TopItem objects. * limit: The limit of how many tags to return. + * cacheable: Whether to cache results. """ - doc = self._request("user.getTopTags", True) + params = self._get_params() + if limit: params["limit"] = limit + + doc = self._request("user.getTopTags", cacheable, params) seq = [] for node in doc.getElementsByTagName("tag"): seq.append(TopItem(Tag(_extract(node, "name"), self.network), _extract(node, "count"))) - if limit: - seq = seq[:limit] - return seq def get_top_tracks(self, period = PERIOD_OVERALL): diff --git a/test_pylast.py b/test_pylast.py index d844856..c7bb82c 100755 --- a/test_pylast.py +++ b/test_pylast.py @@ -100,7 +100,7 @@ class TestPyLast(unittest.TestCase): # Arrange library = pylast.Library(user = self.username, network = self.network) # Pick an artist with plenty of albums - artist = self.network.get_top_artists()[0] + artist = self.network.get_top_artists(limit = 1)[0].item albums = artist.get_top_albums() # Pick a random one to avoid problems running concurrent tests album = choice(albums)[0] @@ -140,7 +140,7 @@ class TestPyLast(unittest.TestCase): # Get plenty of artists artists = self.network.get_top_artists() # Pick a random one to avoid problems running concurrent tests - my_artist = choice(artists) + my_artist = choice(artists).item library = pylast.Library(user = self.username, network = self.network) library.add_artist(my_artist) @@ -824,7 +824,7 @@ class TestPyLast(unittest.TestCase): def test_cacheable_track_get_shouts(self): # Arrange - track = self.network.get_top_tracks()[0] + track = self.network.get_top_tracks()[0].item # Act/Assert self.helper_validate_cacheable(track, "get_shouts") @@ -1058,11 +1058,55 @@ class TestPyLast(unittest.TestCase): # Assert self.assertEqual(type(links), list) self.assertEqual(len(links), 2) - # How permanent are spotify IDs? If they change, make tests more robust self.assertIn("spotify:track:", links[0]) self.assertIn("spotify:track:", links[1]) + def helper_only_one_thing_in_top_list(self, things, expected_type): + # Assert + self.assertEqual(len(things), 1) + self.assertEqual(type(things), list) + self.assertEqual(type(things[0]), pylast.TopItem) + self.assertEqual(type(things[0].item), expected_type) + + def test_user_get_top_tags_with_limit(self): + # Arrange + user = self.network.get_user("RJ") + + # Act + tags = user.get_top_tags(limit = 1) + + # Assert + self.helper_only_one_thing_in_top_list(tags, pylast.Tag) + + + def test_network_get_top_artists_with_limit(self): + # Arrange + # Act + artists = self.network.get_top_artists(limit = 1) + + # Assert + self.helper_only_one_thing_in_top_list(artists, pylast.Artist) + + + def test_network_get_top_tags_with_limit(self): + # Arrange + # Act + tags = self.network.get_top_tags(limit = 1) + + # Assert + self.helper_only_one_thing_in_top_list(tags, pylast.Tag) + + + def test_network_get_top_tracks_with_limit(self): + # Arrange + # Act + tracks = self.network.get_top_tracks(limit = 1) + + # Assert + self.helper_only_one_thing_in_top_list(tracks, pylast.Track) + + if __name__ == '__main__': # For quick testing of a single case (eg. test = "test_scrobble")