Skip to content

Commit 37d60dd

Browse files
authored
Populate extra artists for discogs (#6509)
## Discogs: Populate multi-value contributor fields from artist roles Fixes: #6380 This PR extends the Discogs plugin to properly populate beets' `remixers`, `lyricists`, `composers`, and `arrangers` fields from Discogs artist role credits. ### What changed **`beetsplug/discogs/states.py`** — Core logic change in `ValidArtist` and `ArtistState`: - The `is_feat: bool` field on `ValidArtist` is replaced with `role: str`, preserving the raw Discogs role string rather than discarding it after a single boolean check. - A new `main_artists` property filters to artists with no role or a featuring role — this replaces the previous filtering that happened at parse time, so `valid_artists` now includes _all_ artists regardless of role. - `artists`, `artists_credit`, and `artists_ids` are updated to source from `main_artists` instead of `valid_artists`, ensuring non-primary contributors (e.g. composers, arrangers) no longer appear in artist identity fields. - New properties — `lyricists`, `arrangers`, `remixers`, `composers` — derive their values from `valid_artists` by substring-matching on the `role` field (e.g. `"lyrics"`, `"producer"`, `"remix"`, `"written-by"`). **`beetsplug/discogs/types.py`** — `ArtistInfo` typed dict gains four optional fields: `arrangers`, `composers`, `remixers`, `lyricists`. ### Test changes - `test_anv` is refactored from a flat parametrised function into a `TestAnv` class with a shared `album_info` fixture, splitting the assertions across focused test methods per field group. - `test_parse_featured_artists` gains assertions for `artists_ids` and `composers`. - A new `test_parse_extraartist_roles` test covers role-to-field mapping end-to-end for all four contributor types. ### Impact Discogs imports now populate contributor fields that were previously only filled by the MusicBrainz plugin.
2 parents 80cd215 + bfcddcf commit 37d60dd

File tree

6 files changed

+243
-147
lines changed

6 files changed

+243
-147
lines changed

.github/copilot-instructions.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,23 @@ Core grug beliefs to apply when reviewing:
2020
- Simple APIs good. Layered APIs ok. Java streams make grug reach for club
2121
- SPA frameworks increase complexity demon surface area — be suspicious
2222
- Saying "this too complex for grug" is senior developer superpower — remove Fear Of Looking Dumb (FOLD)
23+
24+
## Pytest conventions in this repo
25+
26+
Before flagging fixture/parametrize issues, remember how pytest resolves names:
27+
28+
- `@pytest.mark.parametrize("foo,bar", [...])` makes `foo` and `bar` behave like
29+
pseudo-fixtures for the entire test call. Any fixture the test depends on
30+
(directly or transitively) can request `foo` in its own signature and
31+
pytest will inject the parametrized value.
32+
- This means a parametrize argname does NOT need to appear in the test
33+
function's own signature, and there does NOT need to be a separate
34+
`@pytest.fixture` defined with that name, as long as some dependent fixture
35+
requests it. `indirect=True` is only required when you want pytest to route
36+
the value through an actual fixture function.
37+
- Concretely, in `test/plugins/test_discogs.py::TestAnv`, `anv_config` is fed
38+
to the `album_info` fixture via this mechanism — this is valid and will
39+
not raise "fixture not found".
40+
41+
When in doubt, check collection with `pytest --collect-only` before claiming
42+
the suite is broken. grug not bark if tests actually pass.

beetsplug/discogs/states.py

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,15 @@ class ValidArtist(NamedTuple):
5151
"""A normalized, render-ready artist entry extracted from Discogs data.
5252
5353
Instances represent the subset of Discogs artist information needed for
54-
tagging, including the join token following the artist and whether the
55-
entry is considered a featured appearance.
54+
tagging, including the join token following the artist and the
55+
lowercased Discogs role string (e.g. "featuring", "written-by", "remix")
5656
"""
5757

5858
id: str
5959
name: str
6060
credit: str
6161
join: str
62-
is_feat: bool
62+
role: str
6363

6464
def get_artist(self, property_name: str) -> str:
6565
"""Return the requested display field with its trailing join token.
@@ -101,7 +101,6 @@ def valid_artists(self) -> list[ValidArtist]:
101101
- substituting the configured 'Various Artists' name when Discogs uses
102102
'Various'
103103
- choosing between name and ANV according to plugin settings
104-
- excluding non-empty roles unless they indicate a featured appearance
105104
- capturing join tokens so the original credit formatting is preserved
106105
"""
107106
va_name = config["va_name"].as_str()
@@ -111,23 +110,26 @@ def valid_artists(self) -> list[ValidArtist]:
111110
self.strip_disambiguation(anv if self.use_anv else name),
112111
self.strip_disambiguation(anv if self.use_credit_anv else name),
113112
a["join"].strip(),
114-
is_feat,
113+
a["role"].lower(),
115114
)
116115
for a in self.raw_artists
117116
if (
118117
(name := va_name if a["name"] == "Various" else a["name"])
119118
and (anv := a["anv"] or name)
120-
and (
121-
(is_feat := ("featuring" in a["role"].lower()))
122-
or not a["role"]
123-
)
124119
)
125120
]
126121

122+
@property
123+
def main_artists(self) -> list[ValidArtist]:
124+
"""Return the per-artist display names used for the 'artist' field."""
125+
return [
126+
a for a in self.valid_artists if not a.role or "featuring" in a.role
127+
]
128+
127129
@property
128130
def artists_ids(self) -> list[str]:
129-
"""Return Discogs artist IDs for all valid artists, preserving order."""
130-
return [a.id for a in self.valid_artists]
131+
"""Return Discogs artist IDs for the main artists, preserving order."""
132+
return [a.id for a in self.main_artists]
131133

132134
@property
133135
def artist_id(self) -> str:
@@ -137,12 +139,12 @@ def artist_id(self) -> str:
137139
@property
138140
def artists(self) -> list[str]:
139141
"""Return the per-artist display names used for the 'artist' field."""
140-
return [a.name for a in self.valid_artists]
142+
return [a.name for a in self.main_artists]
141143

142144
@property
143145
def artists_credit(self) -> list[str]:
144146
"""Return the per-artist display names used for the credit field."""
145-
return [a.credit for a in self.valid_artists]
147+
return [a.credit for a in self.main_artists]
146148

147149
@property
148150
def artist(self) -> str:
@@ -154,6 +156,28 @@ def artist_credit(self) -> str:
154156
"""Return the fully rendered artist credit string."""
155157
return self.join_artists("credit")
156158

159+
@property
160+
def lyricists(self) -> list[str] | None:
161+
return [
162+
a.name for a in self.valid_artists if "lyrics" in a.role
163+
] or None
164+
165+
@property
166+
def arrangers(self) -> list[str] | None:
167+
return [
168+
a.name for a in self.valid_artists if "arrang" in a.role
169+
] or None
170+
171+
@property
172+
def remixers(self) -> list[str] | None:
173+
return [a.name for a in self.valid_artists if "remix" in a.role] or None
174+
175+
@property
176+
def composers(self) -> list[str] | None:
177+
return [
178+
a.name for a in self.valid_artists if "written-by" in a.role
179+
] or None
180+
157181
def join_artists(self, property_name: str) -> str:
158182
"""Render a single artist string with join phrases and featured artists.
159183
@@ -162,8 +186,8 @@ def join_artists(self, property_name: str) -> str:
162186
Discogs order while keeping featured credits separate from the main
163187
artist string.
164188
"""
165-
non_featured = [a for a in self.valid_artists if not a.is_feat]
166-
featured = [a for a in self.valid_artists if a.is_feat]
189+
non_featured = [a for a in self.main_artists if not a.role]
190+
featured = [a for a in self.main_artists if "featuring" in a.role]
167191

168192
artist = "".join(a.get_artist(property_name) for a in non_featured)
169193
if featured:

beetsplug/discogs/types.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ class ArtistInfo(TypedDict):
5555
artists_credit: list[str]
5656
artist_id: str
5757
artists_ids: list[str]
58+
arrangers: list[str] | None
59+
composers: list[str] | None
60+
remixers: list[str] | None
61+
lyricists: list[str] | None
5862

5963

6064
class TracklistInfo(TypedDict):

docs/changelog.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ New features
2828

2929
- :doc:`plugins/inline`: Add access to the ``album`` or ``item`` object as
3030
``db_obj`` in inline fields.
31+
- :doc:`plugins/discogs`: Import Discogs remixer, lyricist, composer, and
32+
arranger credits into the multi-value ``remixers``, ``lyricists``,
33+
``composers``, and ``arrangers`` fields. :bug:`6380`
3134

3235
Bug fixes
3336
~~~~~~~~~

docs/plugins/discogs.rst

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,20 @@ Default
204204
artist: no
205205
album_artist: no
206206
207+
Contributor credits
208+
~~~~~~~~~~~~~~~~~~~
209+
210+
When Discogs provides artist roles on a track, beets uses them to separate main
211+
artists from other credited contributors. Main artist fields such as ``artist``,
212+
``artists``, ``artist_credit``, ``artists_credit``, ``artist_id``, and
213+
``artists_ids`` keep the primary artist credits, while featured artists from
214+
track roles are appended using :conf:`plugins.discogs:featured_string`.
215+
216+
Discogs contributor roles are also imported into beets' multi-value performer
217+
fields when available. This includes remixer, lyricist, composer, and arranger
218+
credits, which populate ``remixers``, ``lyricists``, ``composers``, and
219+
``arrangers`` respectively.
220+
207221
.. include:: ./shared_metadata_source_config.rst
208222

209223
.. _discogs guidelines: https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings

0 commit comments

Comments
 (0)