lastgenre: Add --pretend option for previewing genre changes #6008
Conversation
Reviewer's GuideThis PR introduces a new --pretend flag to the lastgenre plugin, which lets users preview genre assignments without modifying the library. It refactors the genre lookup workflow—renaming temporary variables, improving logging placeholders, and relocating store/write calls under the pretend conditional—and adds corresponding documentation and changelog entries. Sequence diagram for lastgenre command with --pretend optionsequenceDiagram
actor User
participant "lastgenre plugin"
participant Library
User->>"lastgenre plugin": Run lastgenre command with --pretend
"lastgenre plugin"->>Library: Fetch albums/items
"lastgenre plugin"->>"lastgenre plugin": Lookup genre for each album/item
"lastgenre plugin"->>User: Log/display genre suggestions
Note over "lastgenre plugin": No changes are made to Library
Class diagram for updated lastgenre command handlingclassDiagram
class LastGenrePlugin {
+commands()
-_get_genre()
-_log
-sources
+config
}
class lastgenre_func {
+lib
+opts
+args
+write
+pretend
}
LastGenrePlugin <|-- lastgenre_func
LastGenrePlugin : +commands() adds --pretend option
lastgenre_func : +pretend flag controls whether changes are applied
lastgenre_func : genre assignment and store/write calls are conditional on pretend
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a --pretend option to the lastgenre plugin that allows users to preview genre changes without actually applying them to their music library. This provides a safe way to test genre fetching behavior before committing to changes.
Key changes:
- Add command-line
--pretendflag to the lastgenre plugin - Modify genre assignment logic to conditionally apply changes based on pretend mode
- Update logging format to show genre values even when not applied
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/plugins/lastgenre.rst | Documents the new --pretend flag functionality |
| docs/changelog.rst | Adds changelog entry for the new feature |
| beetsplug/lastgenre/init.py | Implements the pretend option with conditional genre assignment |
Comments suppressed due to low confidence (1)
beetsplug/lastgenre/init.py:548
- The conditional logic is incorrect. When pretend mode is active, the album should not be stored at all, but the current logic will still execute
album.store()calls when\"track\" not in self.sources. The store operations should be moved inside theif not pretend:block.
if not pretend:
album.genre = album_genre
if "track" in self.sources:
album.store(inherit=False)
else:
album.store()
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
arsaboo
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The singleton-items branch no longer calls item.try_write(), so disk writes are missing for non-album tracks—restore the original write logic there.
- There’s a lot of repeated “if not pretend” + store/store+write logic; consider extracting it into a helper to keep the code DRY and consistent.
- The log messages mix
{}and numbered placeholders, but Python logging expects%-style formats—please unify to the correct placeholder syntax.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The singleton-items branch no longer calls item.try_write(), so disk writes are missing for non-album tracks—restore the original write logic there.
- There’s a lot of repeated “if not pretend” + store/store+write logic; consider extracting it into a helper to keep the code DRY and consistent.
- The log messages mix `{}` and numbered placeholders, but Python logging expects `%`-style formats—please unify to the correct placeholder syntax.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Hmmm, it's too bad we are working on the same thing at the same time. I guess that's why I made the checklist with all the things I want to bring into lastgenre (mentioning that I have them already implemented);-) I have something pretty similar ready since weeks, but I'm not too happy with it because it introduces a hell load of more if/else clutter in this already polluted plugin. I'm working on something smarte, let's hope I get it done, if not we merge this one :-) |
|
I am happy to close this one....you are much closer to the LG plugin. We can wait for your fix. Look at the positive side....we both felt the need for this :) |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- There’s a lot of repeated
if not pretendlogic around settinggenre/store/try_write—consider extracting that into a small helper or using a guard clause to DRY it up. - The log messages mix
{0.attr}and bare{}placeholders, which can be confusing—pick one formatting style for consistency and clarity. - Since the parser always defines
opts.pretend, you can useopts.pretenddirectly instead ofgetattr(opts, 'pretend', False).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of repeated `if not pretend` logic around setting `genre`/`store`/`try_write`—consider extracting that into a small helper or using a guard clause to DRY it up.
- The log messages mix `{0.attr}` and bare `{}` placeholders, which can be confusing—pick one formatting style for consistency and clarity.
- Since the parser always defines `opts.pretend`, you can use `opts.pretend` directly instead of `getattr(opts, 'pretend', False)`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
I was literally going to request this feature! |
|
Tests are not my strong suit....but here are the logs with lastgenre: genre for album "Achanak Bhayanak" (original fallback): Indi Pop
lastgenre: raw last.fm tags: ['indi pop', 'pop']
lastgenre: existing genres taken into account: ['indi pop']
lastgenre: genre for album "Ye Bhi Woh Bhi" (keep + album artist, whitelist): Indi Pop
lastgenre: last.fm error: Album not found
$ beet info album:"Achanak Bhayanak"
/data/music/Hindi Music/A/Achanak Bhayanak (2021)/Achanak Bhayanak (2021) - Achanak Bhayanak.mp3
album: Achanak Bhayanak
albumartist: 7Bantai'Z
albumtype: single
albumtypes: single
arranger:
art: True
artist: 7Bantai'Z, Bluesanova
bitdepth: 0
bitrate: 198329
bitrate_mode: VBR
bpm: 140
channels: 2
comp: False
date: 2021-07-23
day: 23
disc: 1
disctotal: 1
encoder: Lavf58.29.100
encoder_info:
encoder_settings:
format: MP3
genre: Indi Pop
genres: Indi Pop
label: 7Bantai'Z
length: 253.83183673469387
lyrics:
mb_albumartistid: 0Mz9BTzWvM56Ap38mJ5zpK
mb_albumartistids: 0Mz9BTzWvM56Ap38mJ5zpK
mb_albumid: 1l4GQzsOX2A1LDBsZ0fWe9
mb_artistid: 0Mz9BTzWvM56Ap38mJ5zpK
mb_artistids: 0Mz9BTzWvM56Ap38mJ5zpK
mb_trackid: 1dDvlh2AfyknqYQgS06h8U
month: 7
original_year: 0
samplerate: 44100
title: Achanak Bhayanak
track: 1
tracktotal: 1
year: 2021 |
|
Just a sidenote: Do not worry here about proper logging. In my opinion lines should be logged something like I will add that in #6021 anyway! |
What about prompting an AI to draft a test and we work from there? Review it together here? |
That's a very good idea! What do you think @arsaboo ? |
…nd improve documentation clarity
Co-authored-by: Šarūnas Nejus <[email protected]>
Co-authored-by: Šarūnas Nejus <[email protected]>
and add and original genre instead empty string (clarify intention of test / readability). Remove not really necessary assert items checks.
JOJ0
dismissed
snejus’s stale review
@arsaboo addressed all issues. not sure why github things these requested changes were not fullfilled yet.
Description
Introduce a
--pretendoption to the lastgenre plugin, allowing users to preview genre changes without making any modifications to their library. This feature enhances user control by showing potential changes before they are applied.To Do
docs/to describe it.)docs/changelog.rstto the bottom of one of the lists near the top of the document.)cc: @JOJ0
Summary by Sourcery
Introduce a --pretend option to the lastgenre plugin to allow users to preview genre assignments without altering their library files.
New Features:
Enhancements:
Documentation: