Remove duplication into _extract_top_artists(doc, network)

pylast.py

@@ -363,13 +363,7 @@ class _Network(object):
 
         doc = _Request(self, "chart.getTopArtists", params).execute(cacheable)
 
-        seq = []
+        return _extract_top_artists(doc, self)
-        for node in doc.getElementsByTagName("artist"):
-            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, cacheable=True):
         """Returns the most played tracks as a sequence of TopItem objects."""
@@ -497,16 +491,7 @@ class _Network(object):
 
         doc = _Request(self, "geo.getTopArtists", params).execute(cacheable)
 
-        artists = doc.getElementsByTagName("artist")
+        return _extract_top_artists(doc, self)
-        seq = []
-
-        for artist in artists:
-            name = _extract(artist, "name")
-            listeners = _extract(artist, "listeners")
-
-            seq.append(TopItem(Artist(name, self), listeners))
-
-        return seq
 
     def get_geo_top_tracks(
             self, country, location=None, limit=None, cacheable=True):
@@ -2304,19 +2289,15 @@ class Country(_BaseObject):
 
         return self.name
 
-    def get_top_artists(self):
+    def get_top_artists(self, limit=None, cacheable=True):
         """Returns a sequence of the most played artists."""
+        params = self._get_params()
+        if limit:
+            params['limit'] = limit
 
-        doc = self._request('geo.getTopArtists', True)
+        doc = self._request('geo.getTopArtists', cacheable, params)
 
-        seq = []
+        return _extract_top_artists(doc, self)
-        for node in doc.getElementsByTagName("artist"):
-            name = _extract(node, 'name')
-            playcount = _extract(node, "playcount")
-
-            seq.append(TopItem(Artist(name, self.network), playcount))
-
-        return seq
 
     def get_top_tracks(self, limit=None, cacheable=True):
         """Returns a sequence of the most played tracks"""
@@ -2887,7 +2868,6 @@ class Tag(_BaseObject):
         doc = self._request(self.ws_prefix + '.getTopAlbums', True)
 
         seq = []
-
         for node in doc.getElementsByTagName("album"):
             name = _extract(node, "name")
             artist = _extract(node, "name", 1)
@@ -2906,19 +2886,17 @@ class Tag(_BaseObject):
         return self._get_things(
             "getTopTracks", "track", Track, params, cacheable)
 
-    def get_top_artists(self):
+    def get_top_artists(self, limit=None, cacheable=True):
         """Returns a sequence of the most played artists."""
 
-        doc = self._request(self.ws_prefix + '.getTopArtists', True)
+        params = self._get_params()
+        if limit:
+            params['limit'] = limit
 
-        seq = []
+        doc = self._request(
-        for node in doc.getElementsByTagName("artist"):
+            self.ws_prefix + '.getTopArtists', cacheable, params)
-            name = _extract(node, 'name')
-            playcount = _extract(node, "playcount")
 
-            seq.append(TopItem(Artist(name, self.network), playcount))
+        return _extract_top_artists(doc, self.network)
-
-        return seq
 
     def get_url(self, domain_name=DOMAIN_ENGLISH):
         """Returns the url of the tag page on the network.
@@ -3515,7 +3493,7 @@ class User(_BaseObject):
 
         return seq
 
-    def get_top_artists(self, period=PERIOD_OVERALL):
+    def get_top_artists(self, period=PERIOD_OVERALL, limit=None):
         """Returns the top artists played by a user.
         * period: The period of time. Possible values:
           o PERIOD_OVERALL
@@ -3527,17 +3505,11 @@ class User(_BaseObject):
 
         params = self._get_params()
         params['period'] = period
+        if limit: params["limit"] = limit
 
         doc = self._request(self.ws_prefix + '.getTopArtists', True, params)
 
-        seq = []
+        return _extract_top_artists(doc, self.network)
-        for node in doc.getElementsByTagName('artist'):
-            name = _extract(node, 'name')
-            playcount = _extract(node, "playcount")
-
-            seq.append(TopItem(Artist(name, self.network), playcount))
-
-        return seq
 
     def get_top_tags(self, limit=None, cacheable=True):
         """
@@ -4073,6 +4045,18 @@ def _extract_all(node, name, limit_count=None):
     return seq
 
 
+def _extract_top_artists(doc, network):
+    # TODO Maybe include the _request here too?
+    seq = []
+    for node in doc.getElementsByTagName("artist"):
+        name = _extract(node, "name")
+        playcount = _extract(node, "playcount")
+
+        seq.append(TopItem(Artist(name, network), playcount))
+
+    return seq
+
+
 def _extract_events_from_doc(doc, network):
     events = []
     for node in doc.getElementsByTagName("event"):

test_pylast.py

@@ -1343,6 +1343,36 @@ class TestPyLast(unittest.TestCase):
         self.assertEqual(
             "http://www.lastfm.fr/music/test%2bartist/_/test%2btitle", url)
 
+    def test_tag_top_artists(self):
+        # Arrange
+        tag = self.network.get_tag("blues")
+
+        # Act
+        artists = tag.get_top_artists(limit=1)
+
+        # Assert
+        self.helper_only_one_thing_in_top_list(artists, pylast.Artist)
+
+    def test_country_top_artists(self):
+        # Arrange
+        country = self.network.get_country("Ukraine")
+
+        # Act
+        artists = country.get_top_artists(limit=1)
+
+        # Assert
+        self.helper_only_one_thing_in_top_list(artists, pylast.Artist)
+
+    def test_user_top_artists(self):
+        # Arrange
+        lastfm_user = self.network.get_user(self.username)
+
+        # Act
+        artists = lastfm_user.get_top_artists(limit=1)
+
+        # Assert
+        self.helper_only_one_thing_in_top_list(artists, pylast.Artist)
+
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(