-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
lastgenre: Fix track-level handling and streamline logging #5582
Conversation
When `lastgenre.source: track` is configured, - `lastgenre -a` _should not_ fall back to the album level genre (by making use of the with_album=False kwarg of the Libary's get method). - `lastgenre -a`, when finally storing the genres of _an album_, should _not_ also write the tracks genres (by making use of the inherit=False kwarg of the Album's store method.
It was rather confusing that the lastgenre plugin, when handling singletons, sometimes showed that it applied genres from last.fm and sometimes didn't (it did only in debug log). This streamlines the behaviour: - Change debug to info log. - Streamline wording. - Display details about the track.
is configured.
- Printing out album/item in default format could lead to unreadable clutter depending on the user's configured formats. - The album's name and the individual tracks' title should be just sufficient to provide context as well readability. - Log like this while importing as well as in standalone runs.
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
Ready for final testing if you find the time @arsaboo. Also ready for a final code review @snejus or @Serene-Arc, if you have a minute. I think it's pretty straightforward. Testing like this:
and
|
Tested and here's my output: $ beet lastgenre -A
lastgenre: genre for track Die With A Smile (None):
arsaboo@ubuntuvm2:~$ beet lastgenre -A -s track
lastgenre: genre for track Die With A Smile (None): I guess this PR is working. The tracks not found is probably a separate issue (about Lastfm search that needs to be improved) |
This is best tested with an (not too rare) album with many tracks or even better with a various artist compilation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, merge when ready
Description
Fix
lastgenre -A
in combination with config optionsource: track
(Tracks inherited the album's genre even when this option was set)Fix log-level and message wording being slightly different for
source:
track, album, artist genreNote: This was split out from PR #4982
To Do
keep_existing
#4982 will include a better pytest-based test for the_get_genre
function)