Remove duplication into _extract_top_albums(doc, network)

pylast.py

@@ -2862,20 +2862,16 @@ class Tag(_BaseObject):
 
         return seq
 
-    def get_top_albums(self):
+    def get_top_albums(self, limit=None, cacheable=True):
         """Retuns a list of the top albums."""
+        params = self._get_params()
+        if limit:
+            params['limit'] = limit
 
-        doc = self._request(self.ws_prefix + '.getTopAlbums', True)
+        doc = self._request(
+            self.ws_prefix + '.getTopAlbums', cacheable, params)
 
-        seq = []
+        return _extract_top_albums(doc, self.network)
-        for node in doc.getElementsByTagName("album"):
-            name = _extract(node, "name")
-            artist = _extract(node, "name", 1)
-            playcount = _extract(node, "playcount")
-
-            seq.append(TopItem(Album(artist, name, self.network), playcount))
-
-        return seq
 
     def get_top_tracks(self, limit=None, cacheable=True):
         """Returns a list of the most played Tracks for this tag."""
@@ -3468,7 +3464,8 @@ class User(_BaseObject):
         return doc.getElementsByTagName(
             "registered")[0].getAttribute("unixtime")
 
-    def get_top_albums(self, period=PERIOD_OVERALL):
+    def get_top_albums(
+            self, period=PERIOD_OVERALL, limit=None, cacheable=True):
         """Returns the top albums played by a user.
         * period: The period of time. Possible values:
           o PERIOD_OVERALL
@@ -3480,18 +3477,12 @@ class User(_BaseObject):
 
         params = self._get_params()
         params['period'] = period
+        if limit: params['limit'] = limit
 
-        doc = self._request(self.ws_prefix + '.getTopAlbums', True, params)
+        doc = self._request(
+            self.ws_prefix + '.getTopAlbums', cacheable, params)
 
-        seq = []
+        return _extract_top_albums(doc, self.network)
-        for node in doc.getElementsByTagName('album'):
-            name = _extract(node, 'name')
-            artist = _extract(node, 'name', 1)
-            playcount = _extract(node, "playcount")
-
-            seq.append(TopItem(Album(artist, name, self.network), playcount))
-
-        return seq
 
     def get_top_artists(self, period=PERIOD_OVERALL, limit=None):
         """Returns the top artists played by a user.
@@ -4057,6 +4048,19 @@ def _extract_top_artists(doc, network):
     return seq
 
 
+def _extract_top_albums(doc, network):
+    # TODO Maybe include the _request here too?
+    seq = []
+    for node in doc.getElementsByTagName("album"):
+        name = _extract(node, "name")
+        artist = _extract(node, "name", 1)
+        playcount = _extract(node, "playcount")
+
+        seq.append(TopItem(Album(artist, name, network), playcount))
+
+    return seq
+
+
 def _extract_events_from_doc(doc, network):
     events = []
     for node in doc.getElementsByTagName("event"):

test_pylast.py

@@ -1373,6 +1373,26 @@ class TestPyLast(unittest.TestCase):
         # Assert
         self.helper_only_one_thing_in_top_list(artists, pylast.Artist)
 
+    def test_tag_top_albums(self):
+        # Arrange
+        tag = self.network.get_tag("blues")
+
+        # Act
+        albums = tag.get_top_albums(limit=1)
+
+        # Assert
+        self.helper_only_one_thing_in_top_list(albums, pylast.Album)
+
+    def test_user_top_albums(self):
+        # Arrange
+        user = self.network.get_user("RJ")
+
+        # Act
+        albums = user.get_top_albums(limit=1)
+
+        # Assert
+        self.helper_only_one_thing_in_top_list(albums, pylast.Album)
+
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(