lyrics: move intrumental lyrics to a flexible attribute#6762
Open
snejus wants to merge 2 commits into
Open
Conversation
16c3731 to
bf74cb6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
grug see PR try make lyrics for instrumental track clean. no more store magic text "[Instrumental]" inside item.lyrics. instead store empty lyrics text, and carry “instrumental” state as metadata (lyrics_instrumental) with migration for old libraries.
Changes:
- add
instrumentalflag tobeets.util.lyrics.Lyrics, and normalize legacy"[Instrumental]"into empty text + flag - lyrics plugin now persist
lyrics_instrumentalflex field alongside other lyrics metadata - add DB migration to move legacy
"[Instrumental]"out ofitems.lyricsintolyrics_instrumental, plus docs/tests updates
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
beets/util/lyrics.py |
add instrumental flag + normalize legacy instrumental marker to empty lyrics text |
beetsplug/lyrics.py |
register lyrics_instrumental flex type and persist it from Lyrics object |
beets/library/migrations.py |
new migration to move legacy "[Instrumental]" marker to flex field |
beets/library/library.py |
register new migration in library migration list |
test/util/test_lyrics.py |
update expectations for instrumental normalization + flag |
test/plugins/test_lyrics.py |
update plugin expectations for instrumental behavior |
test/library/test_migrations.py |
add regression test for new migration behavior |
docs/plugins/lyrics.rst |
document new lyrics_instrumental flex attribute + empty lyrics behavior |
docs/changelog.rst |
changelog entry for new instrumental handling + migration |
bf74cb6 to
974aa56
Compare
| item = helper.lib.get_item(item.id) | ||
|
|
||
| assert item.lyrics_url == lyrics.url | ||
| assert item.lyrics_instrumental == "0" |
| regular_item.load() | ||
|
|
||
| assert instrumental_item.lyrics == "" | ||
| assert instrumental_item.lyrics_instrumental == "1" |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6762 +/- ##
==========================================
+ Coverage 74.85% 75.06% +0.20%
==========================================
Files 163 163
Lines 20947 20967 +20
Branches 3299 3301 +2
==========================================
+ Hits 15680 15739 +59
+ Misses 4510 4468 -42
- Partials 757 760 +3
🚀 New features to boost your workflow:
|
f1105fe to
6a8c907
Compare
- Leave lyrics text empty when a backend reports an instrumental track and persist that state in lyrics_instrumental.
6a8c907 to
f7b6c7d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6719
This change shifts instrumental-track handling in the lyrics flow from storing the literal
"[Instrumental]"initem.lyricsto storing that state as structured metadata inlyrics_instrumental.At the architecture level, the source of truth is now the
Lyricsvalue object inbeets/util/lyrics.py: it normalizes instrumental results early, clears the lyrics text, and carries aninstrumentalflag through the rest of the pipeline.The
lyricsplugin then persists that flag as the flexible fieldlyrics_instrumental, alongside existing metadata likelyrics_backendandlyrics_url. This keeps canonical lyrics text clean while preserving whether the track was identified as instrumental.A new library migration,
InstrumentalLyricsInFlexFieldMigration, updates existing databases by moving legacy"[Instrumental]"values out of the mainlyricscolumn and into the flexible attribute. This makes old data match the new model automatically.Docs and tests were updated to reflect the new behavior, so reviewers can think of this as a small data-model cleanup with backward-compatible migration: cleaner
lyricscontent, explicit instrumental metadata, and automatic upgrade for existing libraries.