Conversation
da2d4bb to
b41d349
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6494 +/- ##
==========================================
+ Coverage 70.16% 70.17% +0.01%
==========================================
Files 147 147
Lines 18651 18660 +9
Branches 3039 3040 +1
==========================================
+ Hits 13086 13095 +9
Misses 4923 4923
Partials 642 642
🚀 New features to boost your workflow:
|
85969e8 to
c77ea75
Compare
b41d349 to
13f48d1
Compare
c77ea75 to
a95fea1
Compare
13f48d1 to
6eb483c
Compare
There was a problem hiding this comment.
Pull request overview
PR make beet modify accept old singular multi-value field names, then rewrite to new plural field names and warn user, so user no longer make wrong custom field by accident.
Changes:
- Add central legacy->plural list field maps in
beets/util/deprecation.py - Reuse those maps from autotag
AlbumInfo/TrackInfoinstead of inline maps - Update
modify_parse_argsto rewrite legacy assignment keys + emit user-facing deprecation warning; add tests + docs
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
beets/util/deprecation.py |
Add shared legacy singular → plural list-field mapping constants. |
beets/autotag/hooks.py |
Switch Info subclasses to use shared mapping constants. |
beets/ui/commands/modify.py |
Rewrite legacy assignment keys to plural fields + warn user. |
test/ui/commands/test_modify.py |
Update call sites for new arg + add rewrite/warn tests. |
test/autotag/test_hooks.py |
Stop using Info directly in legacy-field tests; use TrackInfo. |
docs/reference/cli.rst |
Document legacy singular field support in modify. |
docs/changelog.rst |
Add changelog entry for modify legacy-field rewrite behavior. |
| list_field_by_legacy_field = ( | ||
| ALBUM_LIST_FIELD_BY_LEGACY_FIELD | ||
| if is_album | ||
| else ITEM_LIST_FIELD_BY_LEGACY_FIELD | ||
| ) | ||
| for arg in args: | ||
| if arg.endswith("!") and "=" not in arg and ":" not in arg: | ||
| dels.append(arg[:-1]) # Strip trailing !. |
There was a problem hiding this comment.
grug see docs say legacy singular fields (composer/lyricist/remixer/arranger) translate too. but here when is_album True you pick ALBUM_LIST_FIELD_BY_LEGACY_FIELD (only genre). so beet modify -a composer=... still make new composer flex field and not touch composers, so tag write still wrong. grug think modify_parse_args should use item legacy->list map even in album mode (or union maps) so album modify can still rewrite these fields when inherit push to items.
| list_field_by_legacy_field = ( | |
| ALBUM_LIST_FIELD_BY_LEGACY_FIELD | |
| if is_album | |
| else ITEM_LIST_FIELD_BY_LEGACY_FIELD | |
| ) | |
| for arg in args: | |
| if arg.endswith("!") and "=" not in arg and ":" not in arg: | |
| dels.append(arg[:-1]) # Strip trailing !. | |
| list_field_by_legacy_field = dict(ITEM_LIST_FIELD_BY_LEGACY_FIELD) | |
| if is_album: | |
| list_field_by_legacy_field.update(ALBUM_LIST_FIELD_BY_LEGACY_FIELD) | |
| for arg in args: | |
| if arg.endswith("!") and "=" not in arg and ":" not in arg: | |
| key = arg[:-1] # Strip trailing !. | |
| if list_field := list_field_by_legacy_field.get(key): | |
| deprecate_for_user( | |
| log, | |
| f"The '{key}' field", | |
| f"'{list_field}' (separate values by '; ')", | |
| ) | |
| key = list_field | |
| dels.append(key) |
beets/autotag/hooks.py
Outdated
| IGNORED_FIELDS: ClassVar[set[str]] = {"data_url"} | ||
| MEDIA_FIELD_MAP: ClassVar[dict[str, str]] = {} | ||
| LIST_BY_LEGACY_FIELD: ClassVar[dict[str, str]] = {"genre": "genres"} | ||
| LIST_BY_LEGACY_FIELD: ClassVar[dict[str, str]] |
There was a problem hiding this comment.
grug see base Info declare LIST_BY_LEGACY_FIELD but no value. if someone ever make Info() and set any key, self.LIST_BY_LEGACY_FIELD lookup can crash with AttributeError. grug think set safe default (empty dict or genre->genres) on Info class so no surprise crash.
| LIST_BY_LEGACY_FIELD: ClassVar[dict[str, str]] | |
| LIST_BY_LEGACY_FIELD: ClassVar[dict[str, str]] = {} |
a95fea1 to
753c398
Compare
6eb483c to
43e4469
Compare
753c398 to
809cbca
Compare
43e4469 to
b5a59b5
Compare
809cbca to
392962e
Compare
b5a59b5 to
2427ad7
Compare
8997fa5 to
8c57b87
Compare
2427ad7 to
297bcc7
Compare
297bcc7 to
03728f2
Compare
Handle legacy singular field names in
modifyFixes: #6483
The
modifycommand now accepts legacy singular field names (genre,composer,lyricist,remixer,arranger) as assignment targets, automatically rewrites them to their plural multi-value equivalents, and emits a deprecation warning directing users to the correct names.What changed
beets/util/deprecation.py: Two new constants —ALBUM_LIST_FIELD_BY_LEGACY_FIELDandITEM_LIST_FIELD_BY_LEGACY_FIELD— act as a single source of truth for the legacy-to-plural field mappings. Previously these mappings were scattered acrosshooks.py.beets/autotag/hooks.py:AlbumInfoandTrackInfonow reference the centralised constants instead of defining their own inline mappings.beets/ui/commands/modify.py:modify_parse_argsgains anis_albumflag, selects the appropriate mapping, and — when a legacy field is detected — rewrites the key and warns the user viadeprecate_for_user.modify_parse_argscall-sites updated to passis_album; new parametrised tests cover the rewrite and warning behaviour.Impact
Users who run commands like
beet modify genre="rock"will now get a working command plus a clear prompt to switch togenres="rock", rather than silently writing to a non-standard field or failing. No breaking change — purely additive compatibility layer.