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

@@ -334,51 +334,55 @@ class _Network(object):
 
         return Playlist(user, e_id, self)
 
-    def get_top_artists(self, limit=None):
+    def get_top_artists(self, limit=None, cacheable=True):
-        """Returns a sequence of the most played artists."""
+        """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(_extract(node, "name"), self)
-            artist = Artist(title, self)
+            weight = _number(_extract(node, "playcount"))
-            seq.append(artist)
+            seq.append(TopItem(artist, weight))
-
-        if limit:
-            seq = seq[:limit]
 
         return seq
 
-    def get_top_tracks(self, limit=None):
+    def get_top_tracks(self, limit=None, cacheable=True):
-        """Returns a sequence of the most played tracks."""
+        """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)
+            weight = _number(_extract(node, "playcount"))
-
+            seq.append(TopItem(track, weight))
-        if limit:
-            seq = seq[:limit]
 
         return seq
 
-    def get_top_tags(self, limit=None):
+    def get_top_tags(self, limit=None, cacheable=True):
-        """Returns a sequence of the most used tags as a sequence of TopItem objects."""
+        """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):

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")