From 08274028ebf1f3c611e61e23440df88334377455 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:01:12 +0200 Subject: [PATCH 1/6] Set limit to 50 by default, not 1 --- src/pylast/__init__.py | 2 +- tests/test_artist.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/pylast/__init__.py b/src/pylast/__init__.py index e02c764..2f527c0 100644 --- a/src/pylast/__init__.py +++ b/src/pylast/__init__.py @@ -1149,7 +1149,7 @@ class _BaseObject: def _get_things(self, method, thing, thing_type, params=None, cacheable=True): """Returns a list of the most played thing_types by this thing.""" - limit = params.get("limit", 1) + limit = params.get("limit", 50) seq = [] for node in _collect_nodes( limit, self, self.ws_prefix + "." + method, cacheable, params diff --git a/tests/test_artist.py b/tests/test_artist.py index 679d917..69dafb9 100755 --- a/tests/test_artist.py +++ b/tests/test_artist.py @@ -131,6 +131,17 @@ class TestPyLastArtist(TestPyLastWithLastFm): # Assert assert len(things) == 100 + def test_artist_top_albums_limit_default(self): + # Arrange + # Pick an artist with plenty of plays + artist = self.network.get_top_artists(limit=1)[0].item + + # Act + things = artist.get_top_albums() + + # Assert + assert len(things) == 50 + def test_artist_listener_count(self): # Arrange artist = self.network.get_artist("Test Artist") From a5034c43c043f0ff8caf9463c02b76bff30a4001 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:05:52 +0200 Subject: [PATCH 2/6] Refactor with pytest.mark.parametrize --- tests/test_artist.py | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/tests/test_artist.py b/tests/test_artist.py index 69dafb9..c99b04e 100755 --- a/tests/test_artist.py +++ b/tests/test_artist.py @@ -95,41 +95,17 @@ class TestPyLastArtist(TestPyLastWithLastFm): # Assert self.helper_two_different_things_in_top_list(things, pylast.Album) - def test_artist_top_albums_limit_1(self): + @pytest.mark.parametrize("test_limit", [1, 50, 100]) + def test_artist_top_albums_limit(self, test_limit: int) -> None: # Arrange - limit = 1 # Pick an artist with plenty of plays artist = self.network.get_top_artists(limit=1)[0].item # Act - things = artist.get_top_albums(limit=limit) + things = artist.get_top_albums(limit=test_limit) # Assert - assert len(things) == 1 - - def test_artist_top_albums_limit_50(self): - # Arrange - limit = 50 - # Pick an artist with plenty of plays - artist = self.network.get_top_artists(limit=1)[0].item - - # Act - things = artist.get_top_albums(limit=limit) - - # Assert - assert len(things) == 50 - - def test_artist_top_albums_limit_100(self): - # Arrange - limit = 100 - # Pick an artist with plenty of plays - artist = self.network.get_top_artists(limit=1)[0].item - - # Act - things = artist.get_top_albums(limit=limit) - - # Assert - assert len(things) == 100 + assert len(things) == test_limit def test_artist_top_albums_limit_default(self): # Arrange From 23503a72128c93b726324a345a47a1f9f03e6c3a Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:22:20 +0200 Subject: [PATCH 3/6] Refactor to remove unused parameter --- src/pylast/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pylast/__init__.py b/src/pylast/__init__.py index 2f527c0..e53d9da 100644 --- a/src/pylast/__init__.py +++ b/src/pylast/__init__.py @@ -1146,8 +1146,8 @@ class _BaseObject: return first_child.wholeText.strip() - def _get_things(self, method, thing, thing_type, params=None, cacheable=True): - """Returns a list of the most played thing_types by this thing.""" + def _get_things(self, method, thing_type, params=None, cacheable=True): + """Returns a list of the most played thing_types.""" limit = params.get("limit", 50) seq = [] @@ -1812,7 +1812,7 @@ class Artist(_BaseObject, _Taggable): if limit: params["limit"] = limit - return self._get_things("getTopAlbums", "album", Album, params, cacheable) + return self._get_things("getTopAlbums", Album, params, cacheable) def get_top_tracks(self, limit=None, cacheable=True): """Returns a list of the most played Tracks by this artist.""" @@ -1820,7 +1820,7 @@ class Artist(_BaseObject, _Taggable): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_url(self, domain_name=DOMAIN_ENGLISH): """Returns the URL of the artist page on the network. @@ -1894,7 +1894,7 @@ class Country(_BaseObject): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_url(self, domain_name=DOMAIN_ENGLISH): """Returns the URL of the country page on the network. @@ -2024,7 +2024,7 @@ class Tag(_BaseObject, _Chartable): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_top_artists(self, limit=None, cacheable=True): """Returns a sequence of the most played artists.""" @@ -2498,7 +2498,7 @@ class User(_BaseObject, _Chartable): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_track_scrobbles(self, artist, track, cacheable=False): """ From 7327b303371a653c71bae3a77254b2166028fe82 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:22:41 +0200 Subject: [PATCH 4/6] Refactor to avoid shadowing built-in --- tests/test_pylast.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_pylast.py b/tests/test_pylast.py index 789afad..c734d32 100755 --- a/tests/test_pylast.py +++ b/tests/test_pylast.py @@ -34,11 +34,11 @@ def load_secrets(): # pragma: no cover class PyLastTestCase: - def assert_startswith(self, str, prefix, start=None, end=None): - assert str.startswith(prefix, start, end) + def assert_startswith(self, s, prefix, start=None, end=None): + assert s.startswith(prefix, start, end) - def assert_endswith(self, str, suffix, start=None, end=None): - assert str.endswith(suffix, start, end) + def assert_endswith(self, s, suffix, start=None, end=None): + assert s.endswith(suffix, start, end) def _no_xfail_rerun_filter(err, name, test, plugin): From 0999501600fd7c3fff68b07aa93d8d894f03ac9a Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Tue, 29 Dec 2020 21:24:05 +0200 Subject: [PATCH 5/6] Fix comment --- src/pylast/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pylast/__init__.py b/src/pylast/__init__.py index c4f3541..fc71011 100644 --- a/src/pylast/__init__.py +++ b/src/pylast/__init__.py @@ -2321,7 +2321,7 @@ class User(_Chartable): reverse order of their timestamp, all the way back to the first track. If limit==None, it will try to pull all the available data. - If stream=False, it will yield tracks as soon as a page has been retrieved. + If stream=True, it will yield tracks as soon as a page has been retrieved. This method uses caching. Enable caching only if you're pulling a large amount of data. From 8be8c4efb6905ef97e18f2ecd75b95fd9f67e08c Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Tue, 29 Dec 2020 21:32:29 +0200 Subject: [PATCH 6/6] No need to set param with default --- tests/test_artist.py | 6 +++--- tests/test_network.py | 4 ++-- tests/test_pylast.py | 4 ++-- tests/test_user.py | 22 ++++++++++------------ 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/tests/test_artist.py b/tests/test_artist.py index befa778..e389010 100755 --- a/tests/test_artist.py +++ b/tests/test_artist.py @@ -78,7 +78,7 @@ class TestPyLastArtist(TestPyLastWithLastFm): artist = self.network.get_top_artists(limit=1)[0].item # Act - things = artist.get_top_tracks(limit=2, stream=False) + things = artist.get_top_tracks(limit=2) # Assert self.helper_two_different_things_in_top_list(things, pylast.Track) @@ -101,7 +101,7 @@ class TestPyLastArtist(TestPyLastWithLastFm): artist = self.network.get_top_artists(limit=1)[0].item # Act - things = artist.get_top_albums(limit=limit, stream=False) + things = artist.get_top_albums(limit=limit) # Assert assert len(things) == 1 @@ -113,7 +113,7 @@ class TestPyLastArtist(TestPyLastWithLastFm): artist = self.network.get_top_artists(limit=1)[0].item # Act - things = artist.get_top_albums(limit=limit, stream=False) + things = artist.get_top_albums(limit=limit) # Assert assert len(things) == 50 diff --git a/tests/test_network.py b/tests/test_network.py index bdf9435..cebc7c6 100755 --- a/tests/test_network.py +++ b/tests/test_network.py @@ -153,7 +153,7 @@ class TestPyLastNetwork(TestPyLastWithLastFm): country = self.network.get_country("Croatia") # Act - things = country.get_top_tracks(limit=2, stream=False) + things = country.get_top_tracks(limit=2) # Assert self.helper_two_different_things_in_top_list(things, pylast.Track) @@ -171,7 +171,7 @@ class TestPyLastNetwork(TestPyLastWithLastFm): tag = self.network.get_tag("blues") # Act - things = tag.get_top_tracks(limit=2, stream=False) + things = tag.get_top_tracks(limit=2) # Assert self.helper_two_different_things_in_top_list(things, pylast.Track) diff --git a/tests/test_pylast.py b/tests/test_pylast.py index b17fc0c..da5d816 100755 --- a/tests/test_pylast.py +++ b/tests/test_pylast.py @@ -94,8 +94,8 @@ class TestPyLastWithLastFm(PyLastTestCase): func = getattr(thing, function_name, None) # Act - result1 = func(limit=1, cacheable=False, stream=False) - result2 = func(limit=1, cacheable=True, stream=False) + result1 = func(limit=1, cacheable=False) + result2 = func(limit=1, cacheable=True) result3 = list(func(limit=1)) # Assert diff --git a/tests/test_user.py b/tests/test_user.py index ddf1509..2b1d8fc 100755 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -143,10 +143,10 @@ class TestPyLastUser(TestPyLastWithLastFm): user = self.network.get_user("test-user") # Act/Assert - assert len(user.get_loved_tracks(limit=20, stream=False)) == 20 - assert len(user.get_loved_tracks(limit=100, stream=False)) <= 100 - assert len(user.get_loved_tracks(limit=None, stream=False)) >= 23 - assert len(user.get_loved_tracks(limit=0, stream=False)) >= 23 + assert len(user.get_loved_tracks(limit=20)) == 20 + assert len(user.get_loved_tracks(limit=100)) <= 100 + assert len(user.get_loved_tracks(limit=None)) >= 23 + assert len(user.get_loved_tracks(limit=0)) >= 23 def test_user_is_hashable(self): # Arrange @@ -211,7 +211,7 @@ class TestPyLastUser(TestPyLastWithLastFm): lastfm_user = self.network.get_user("RJ") # Act - things = lastfm_user.get_top_tracks(limit=2, stream=False) + things = lastfm_user.get_top_tracks(limit=2) # Assert self.helper_two_different_things_in_top_list(things, pylast.Track) @@ -362,9 +362,7 @@ class TestPyLastUser(TestPyLastWithLastFm): utc_end = calendar.timegm(end.utctimetuple()) # Act - tracks = lastfm_user.get_recent_tracks( - time_from=utc_start, time_to=utc_end, stream=False - ) + tracks = lastfm_user.get_recent_tracks(time_from=utc_start, time_to=utc_end) # Assert assert len(tracks) == 1 @@ -382,7 +380,7 @@ class TestPyLastUser(TestPyLastWithLastFm): # Act tracks = lastfm_user.get_recent_tracks( - time_from=utc_start, time_to=utc_end, limit=None, stream=False + time_from=utc_start, time_to=utc_end, limit=None ) # Assert @@ -469,7 +467,7 @@ class TestPyLastUser(TestPyLastWithLastFm): user = self.network.get_user("bbc6music") # Act - scrobbles = user.get_track_scrobbles(artist, title, stream=False) + scrobbles = user.get_track_scrobbles(artist, title) # Assert assert len(scrobbles) > 0 @@ -483,7 +481,7 @@ class TestPyLastUser(TestPyLastWithLastFm): user = self.network.get_user("bbc6music") # Act - result1 = user.get_track_scrobbles(artist, title, cacheable=False, stream=False) + result1 = user.get_track_scrobbles(artist, title, cacheable=False) result2 = list(user.get_track_scrobbles(artist, title, cacheable=True)) result3 = list(user.get_track_scrobbles(artist, title)) @@ -500,4 +498,4 @@ class TestPyLastUser(TestPyLastWithLastFm): match="Deprecated - This type of request is no longer supported", ): warnings.filterwarnings("ignore", category=DeprecationWarning) - lastfm_user.get_artist_tracks(artist="Test Artist", stream=False) + lastfm_user.get_artist_tracks(artist="Test Artist")