Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User.get_now_playing: Add album and cover image to info #330

Merged
merged 6 commits into from Jun 22, 2020

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Jun 1, 2020

Fixes #329.

Changes proposed in this pull request:

@hugovk hugovk added enhancement New feature or request changelog: Added For new features labels Jun 1, 2020
@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #330 into master will increase coverage by 0.48%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
+ Coverage   94.44%   94.93%   +0.48%     
==========================================
  Files           1        1              
  Lines        1261     1264       +3     
==========================================
+ Hits         1191     1200       +9     
+ Misses         70       64       -6     
Impacted Files Coverage Δ
src/pylast/__init__.py 94.93% <100.00%> (+0.48%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 898a8b5...6f62857. Read the comment docs.

@vervaekejonathan
Copy link

vervaekejonathan commented Jun 1, 2020

I've tested this, and it adds the cover-art images, but not the album. Is it possible that this function needs to be replaced with the code below: https://github.com/pylast/pylast/blob/add-album-and-image-to-get_now_playing/src/pylast/__init__.py#L2129

    def get_album(self):
        """Returns the album object of this track."""
        if "album" not in self.info:
            doc = self._request(self.ws_prefix + ".getInfo", True)

            albums = doc.getElementsByTagName("album")

            if len(albums) == 0:
                return

            node = doc.getElementsByTagName("album")[0]
            return Album(_extract(node, "artist"), _extract(node, "title"), self.network)
        return Album(self.artist, self.info["album"], self.network)

vervaekejonathan@a12a00a

@hugovk
Copy link
Member Author

hugovk commented Jun 1, 2020

Ah yes, something like that would be useful too.

I had checked like:

current_track = lastfm_user.get_now_playing()

from pprint import pprint
pprint(current_track.info)
print(current_track.info["image"][pylast.SIZE_LARGE])
print(current_track.info["album"])

But it would be good to check if we already saved info["album"] for later calls to current_track.get_album().

So something like the below, which flips the if and returns early if "album" in self.info.

But also, self.info["album"] could exist and be None`. Then we don't want return early, we want to look up on Last.fm in case they still have it.

So return early if "album" in self.info and self.info["album"] is not None, otherwise do as before:

    def get_album(self):
        """Returns the album object of this track."""

        # THESE TWO LINES ARE NEW
        if "album" in self.info and self.info["album"] is not None:
            return Album(self.artist, self.info["album"], self.network)

        doc = self._request(self.ws_prefix + ".getInfo", True)

        albums = doc.getElementsByTagName("album")

        if len(albums) == 0:
            return

        node = doc.getElementsByTagName("album")[0]
        return Album(_extract(node, "artist"), _extract(node, "title"), self.network)

What do you think?

@hugovk hugovk merged commit 108e3dd into master Jun 22, 2020
@hugovk hugovk deleted the add-album-and-image-to-get_now_playing branch June 22, 2020 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: Added For new features enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_now_playing does not contain album and coverart
2 participants