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

Random album playback plugin - make it Python 3 compatible #2922

Merged
merged 4 commits into from Jul 31, 2018

Conversation

Projects
None yet
2 participants
@sphh
Contributor

sphh commented Jul 25, 2018

When using the Random album playback plug-in on Linux Mint 19 with Python 3.6.5 I always get exception when QL switches the album. This was due to the line 193 in quodlibet/ext/events/randomalbum.py:

                album_scores = sorted(self._score(chosen_albums))

The exception was cause by Python 3 not being able to sort class instances without a __le__ method.

The original code sorted the albums and picked the maximum. I changed the code to find the maximum score, filter the albums by this maximum score and pick an album by random from the resulting list. This also solved the problem, that the original randomalbum.py always picked the last album (in alphabetical order), if more than one album has the maximum score.

@frestr

Thanks for the PR! Looks good to me.

The only thing would be the debug print. If you can fix that it'll be good to go.

@@ -190,10 +190,17 @@ def plugin_on_song_started(self, song):
nr_albums = min(total, max(int(0.03 * total), 3))
print_d("Choosing from %d library albums. Best:" % nr_albums)
chosen_albums = random.sample(values, nr_albums)
album_scores = sorted(self._score(chosen_albums))
album_scores = self._score(chosen_albums)
for score, album in album_scores[-5:]:
print_d("%0.1f scored by %s" % (score, album("album")))

This comment has been minimized.

@frestr

frestr Jul 29, 2018

Member

This print should probably be removed, since without the sort it just prints five random albums as opposed to the highest rated ones (the print above would also need to be modified a little, removing the 'Best'). Perhaps print the chosen album instead.

Implemented changes requested by frestr:
- Better debug messages.
@frestr

This comment has been minimized.

Member

frestr commented Jul 31, 2018

Thanks. You'll need to fix the failing tests though.

@frestr

frestr approved these changes Jul 31, 2018

@frestr frestr merged commit 56fd9c8 into quodlibet:master Jul 31, 2018

6 checks passed

ci/circleci: job.fedora28 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu16.04 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu18.04 Your tests passed on CircleCI!
Details
ci/circleci: job.win32 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment