Handle "join keyword" in `MetadataSourcePlugin.get_artist()` #4515

JOJ0 · 2022-10-10T07:58:16Z

Description

Handle "join keyword" in MetadataSourcePlugin.get_artist() and make use of the new functionality in the Discogs source plugin.
Fixes Discogs plugin replacing Feat. or Ft. with a comma #4401 (Note that despite being discussed here, no version bump of python3-discogs-client is required since the data attribute being used, contains the required "join" field already.)

To Do

~~Documentation.~~
Changelog.
~~Tests.~~

JOJ0 · 2022-10-10T08:14:45Z

Some example outputs while importing:

Old behaviour:

(beets)    ~/git/beets   fix_discogs_multi_artist ●  beet import ~/Sync/beets_test_music/Good\ Looking\ -\ Makoto\ Featuring\ MC\ Conrad

/home/jojo/Sync/beets_test_music/Good Looking - Makoto Featuring MC Conrad (2 items)
Finding tags for album "MC Conrad, Makoto - Golden Girl".
Candidates:
1. MC Conrad, Makoto - Golden Girl (67.4%) (id, source) (Discogs, Vinyl, 2005, UK, Good Looking Records, GLR066-12)
2. MC Conrad, Makoto - Golden Girl (56.4%) (id, tracks, source, ...) (Discogs, Vinyl, 2004, UK, Good Looking Records, GLR066W-12)
3. MC Conrad & Makoto - Golden Girl (54.2%) (id, tracks, media, ...) (12" Vinyl, 2005, GB, Good Looking Records, GLR066-12)
4. MC Lars - Hipster Girl (26.7%) (id, artist, album, ...) (2007, XW)
5. MC Conrad - Vocalist 01 (21.4%) (missing tracks, id, tracks, ...) (CD, 2000, GB, Good Looking Records, GLRV01)
6. MC Conrad - Logical Progression Level 4 (14.4%) (missing tracks, id, tracks, ...) (2xCD, 2001, GB, Good Looking Records, GLRCD004)
7. MC Conrad - Logical Progression Level 4 (11.3%) (missing tracks, id, tracks, ...) (3xCD, 2001, GB, Good Looking Records, GLRCD004X)
8. LTJ Bukem, MC Conrad - Progression Sessions 10 - Germany Live 2004 (9.3%) (missing tracks, id, tracks, ...) (Discogs, 2xCD, 2004, UK, Good Looking Records, GLRPS010X-2)
9. LTJ Bukem, MC Conrad - Progression Sessions 10 - Germany Live 2004 (6.9%) (missing tracks, id, tracks, ...) (Discogs, 3xCD, 2004, US, Benz Street, BZ 10017-2)
10. Various Artists - Makoto Feat. Deeizm MC: Non Stop Live Mix @ Tokyo (6.6%) (missing tracks, id, tracks, ...) (Discogs, 2xCD, 2006, Japan, Fenomeno Inc., FENO-0006)
# selection (default 1), Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates? b
(beets)    ~/git/beets   fix_discogs_multi_artist ● 

New behaviour:

(beets)    ~/git/beets   fix_discogs_multi_artist ●  beet import ~/Sync/beets_test_music/Good\ Looking\ -\ Makoto\ Featuring\ MC\ Conrad

/home/jojo/Sync/beets_test_music/Good Looking - Makoto Featuring MC Conrad (2 items)
Finding tags for album "MC Conrad, Makoto - Golden Girl".
Candidates:
1. MC Conrad & Makoto - Golden Girl (74.3%) (id, artist) (Discogs, Vinyl, 2005, UK, Good Looking Records, GLR066-12)
2. MC Conrad And Makoto - Golden Girl (63.2%) (id, tracks, artist, ...) (Discogs, Vinyl, 2004, UK, Good Looking Records, GLR066W-12)
3. MC Conrad & Makoto - Golden Girl (54.2%) (id, tracks, media, ...) (12" Vinyl, 2005, GB, Good Looking Records, GLR066-12)
4. いきものがかり - GOLDEN GIRL (30.5%) (id, tracks, artist, ...) (CD, 2014, TW, Epic Records, 88875045072)
5. MC Conrad - Vocalist 01 (21.4%) (missing tracks, id, tracks, ...) (CD, 2000, GB, Good Looking Records, GLRV01)
6. MC Conrad - Logical Progression Level 4 (14.4%) (missing tracks, id, tracks, ...) (2xCD, 2001, GB, Good Looking Records, GLRCD004)
7. LTJ Bukem Featuring MC Conrad - Progression Sessions 10 - Germany Live 2004 (13.9%) (missing tracks, id, tracks, ...) (Discogs, 2xCD, 2004, UK, Good Looking Records, GLRPS010X-2)
8. Various Artists - Makoto Feat. Deeizm MC: Non Stop Live Mix @ Tokyo (12.0%) (missing tracks, id, tracks, ...) (Discogs, 2xCD, 2006, Japan, Fenomeno Inc., FENO-0006)
9. MC Conrad - Logical Progression Level 4 (11.3%) (missing tracks, id, tracks, ...) (3xCD, 2001, GB, Good Looking Records, GLRCD004X)
10. LTJ Bukem Featuring MC Conrad - Progression Sessions 10 - Germany Live 2004 (11.2%) (missing tracks, id, tracks, ...) (Discogs, 3xCD, 2004, US, Benz Street, BZ 10017-2)
# selection (default 1), Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates? b
(beets)    ~/git/beets   fix_discogs_multi_artist ● 

Another example of new behaviour:

(beets)    ~/git/beets   fix_discogs_multi_artist ●  beet import ~/Sync/beets_test_music/DJ\ Marky\ -\ LK/

/home/jojo/Sync/beets_test_music/DJ Marky - LK (2 items)
Finding tags for album "DJ Marky & XRS, Stamina MC - LK Carolina Carol Bela".
Candidates:
1. DJ Marky & XRS Featuring Stamina MC - LK 'Carolina Carol Bela' (72.7%) (id, artist, tracks) (Discogs, Vinyl, 2002, UK, V Recordings, V035)
2. DJ Marky & XRS Featuring Stamina MC - LK 'Carolina Carol Bela' (64.2%) (id, tracks, artist) (Discogs, Vinyl, 2002, UK, V Recordings, V035)
3. DJ Marky & XRS feat. Stamina MC - LK 'Carolina Carol Bela' (54.9%) (id, tracks, media, ...) (12" Vinyl, 2002, GB, V Recordings, V035)
4. DJ Marky & XRS - LK 'Carolina Carol Bela' (53.5%) (id, tracks, artist, ...) (Discogs, CDr, 2002, UK, V Recordings, VCD035)
5. DJ Marky & XRS featuring Stamina MC - LK 'Carolina Carol Bela' (52.9%) (id, tracks, missing tracks, ...) (Discogs, CD, 2002, UK, V Recordings, V035CD)
6. DJ Marky & XRS - LK 'Carolina Carol Bela' (51.0%) (id, tracks, artist, ...) (12" Vinyl, 2002, GB, V Recordings, V 035)
7. DJ Marky & XRS feat. Stamina MC - LK 'Carolina Carol Bela' (49.0%) (id, tracks, media, ...) (CD, 2002, GB, V Recordings, V035CD)
8. DJ Marky & XRS - LK (remix) / Love Fantasy (35.0%) (id, tracks, album, ...) (12" Vinyl, 2002, GB, V Recordings, V 035R)
9. DJ Marky & XRS - Butterfly / Moments of Lust (remixes) (26.9%) (id, tracks, album, ...) (12" Vinyl, 2006, GB, Innerground Records, INN014)
10. Goldie - Drum&BassArena - The Classics (9.3%) (missing tracks, id, tracks, ...) (Discogs, 2xCD, 2005, UK, Ministry Of Sound, DABA01)
# selection (default 1), Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates? b

JOJ0 · 2022-10-15T07:35:54Z

Ready for a final review.

zaptrem · 2022-12-26T04:38:21Z

Excited for this PR!

sampsyo

Nice!! This is looking good overall—please see one suggestion within.

sampsyo · 2022-12-29T00:58:46Z

beets/plugins.py

+        else:
+            artist = ', '.join(artist_names).replace(' ,', ',') or None


I may have this wrong, but would it work to move this "fallback" up into the loop? That is, the loop could look like this:

if join_key and artist.get(join_key, None): ... else: name += ', '

I'm honestly not 100% sure where the replace call should go… it's a little mysterious to me why a , would appear in the string in the first place.

The reason I suggest this is that it could deal better with a situation where some artists have a join_key value and others don't (when there are more than 2 artists). In that case, you'd probably want to use a comma for pairs of artists that would otherwise not have another joiner.

Thanks a lot for the suggestion. Well, I think I managed to massively simplify this, which I think makes this method much more readable. It works well with Discogs as the metadata source, but some more testing reveals that it's problematic with the beatport4 plugin. Here we don't have a simple list to handle but a generator which does not answer well on my len() call:

File "/home/jojo/git/beets-beatport4/beetsplug/beatport4.py", line 629, in _get_artist return MetadataSourcePlugin.get_artist( File "/home/jojo/git/beets/beets/plugins.py", line 697, in get_artist if idx < (len(artists) - 1): # Skip joining on last. TypeError: object of type 'generator' has no len()

I need to find another way of finding out whether I'm handling the last object of artists.... Suggestions welcome.

I found a way to count the items in a way generators as well as dicts and lists or any other iterable should be allowed to be passed as the artists object.

Still this is better and more readable than my first version? What do you think?

And btw. I forgot to mention before: I removed the "mysterious" string replace method. It was mysterious to me as well why this was there in the original get_artist() code. I would like to leave it out. I don't see the point of keeping it. It probably would just lead to confusion again in the future :-) You agree?

Add an optional argument to MetadataSourcePlugin.get_artist method that enables making use of a field containing a keyword supposed to be used to combine artists together into a single string like "Feat.", "And", "Vs." and so on.

Fixes beetbox#4401

JOJ0 · 2023-01-02T12:57:49Z

Pulled in master because of the recent Sphinx issue. All checks green again and ready for a final review @sampsyo. Thanks!

sampsyo

Nice; looking good!

I did a heap of git blame digging and traced the origin of that .replace call back to the discogs plugin here:
3eb6f8e

That still doesn't really explain why it was needed in the first place. I think we are safe to drop it and, if it crops back up because of some weird Discogs data formatting issues, we can reexamine on a case-by-case basis.

sampsyo · 2023-01-02T23:35:54Z

beets/plugins.py

-        for artist in artists:
+        artist_string = ""
+        # Count items (supports generators as well as lists, dicts, ...)
+        total = sum(1 for _ in artists)


A different option for dealing with generators (iteratables, in general) passed here would be to eagerly convert the artists to a list. That is, you could do:

artists_list = list(artists)

…and then simply use len(artists_list).

Will do that. Thanks!

I just realilsed that my way of solving it didn't work anyway! Sorry for not testing better. It seems a generator is exhausted when it was counted with the sum(...) hack (it's been looped through to the end). I can't loop through it then because it's empty! Alright, sorry again, will fix soon.

Yep, that's exactly the case!

sampsyo · 2023-01-02T23:36:28Z

beets/plugins.py

+            # Use a join keyword if requested and available.
+            if idx < (total - 1):  # Skip joining on last.
+                if join_key and artist.get(join_key, None):
+                    name += " " + artist[join_key] + " "


Consider f" {artist[join_key]} " for legibility.

- Originally a list of "artists" was generated and joined together to a final string later. - This simplifies by concatinating to a final string within the main loop. - Also this commit gets rid of a mysterious replacement code (` ,' -> ',')

JOJ0 · 2023-01-03T08:16:36Z

I put both suggested fixes into the original commits to keep commit-clutter minimal. I think we are done :-)

sampsyo · 2023-01-06T18:30:20Z

That does it! Nice work!!

JOJ0 added a commit to JOJ0/beets that referenced this pull request Oct 10, 2022

Changelog for beetbox#4515

501229c

JOJ0 added a commit to JOJ0/beets that referenced this pull request Oct 30, 2022

Changelog for beetbox#4515

834d9d2

sampsyo reviewed Dec 29, 2022

View reviewed changes

JOJ0 force-pushed the fix_discogs_multi_artist branch 3 times, most recently from fab1d77 to c62a6a6 Compare December 30, 2022 09:58

JOJ0 added 3 commits January 2, 2023 13:47

Add join_key arg to source plugin get_artist method

fddeecb

Add an optional argument to MetadataSourcePlugin.get_artist method that enables making use of a field containing a keyword supposed to be used to combine artists together into a single string like "Feat.", "And", "Vs." and so on.

Use join_key arg in Discogs source plugin

29ed4ab

Fixes beetbox#4401

Changelog for beetbox#4515

e1d6110

JOJ0 force-pushed the fix_discogs_multi_artist branch from c62a6a6 to 75209d3 Compare January 2, 2023 12:47

sampsyo reviewed Jan 2, 2023

View reviewed changes

JOJ0 force-pushed the fix_discogs_multi_artist branch from 75209d3 to 5df959e Compare January 3, 2023 08:00

JOJ0 added 2 commits January 3, 2023 09:08

Support generators in MetadataSourcePlugin.get_artist method

f1794e8

JOJ0 force-pushed the fix_discogs_multi_artist branch from 5df959e to f1794e8 Compare January 3, 2023 08:08

sampsyo merged commit 6989bce into beetbox:master Jan 6, 2023

This was referenced Feb 4, 2023

discogs: append 'Featuring' artists to 'artist' #3363

Closed

discogs: add 'Featuring' artists to the 'artist' field #3353

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle "join keyword" in `MetadataSourcePlugin.get_artist()` #4515

Handle "join keyword" in `MetadataSourcePlugin.get_artist()` #4515

JOJ0 commented Oct 10, 2022 •

edited

Loading

JOJ0 commented Oct 10, 2022

JOJ0 commented Oct 15, 2022

zaptrem commented Dec 26, 2022

sampsyo left a comment

sampsyo Dec 29, 2022

JOJ0 Dec 29, 2022 •

edited

Loading

JOJ0 Dec 29, 2022 •

edited

Loading

JOJ0 commented Jan 2, 2023

sampsyo left a comment

sampsyo Jan 2, 2023

JOJ0 Jan 3, 2023

sampsyo Jan 3, 2023

sampsyo Jan 2, 2023

JOJ0 commented Jan 3, 2023

sampsyo commented Jan 6, 2023

		else:
		artist = ', '.join(artist_names).replace(' ,', ',') or None

Handle "join keyword" in MetadataSourcePlugin.get_artist() #4515

Handle "join keyword" in MetadataSourcePlugin.get_artist() #4515

Conversation

JOJ0 commented Oct 10, 2022 • edited Loading

Description

To Do

JOJ0 commented Oct 10, 2022

JOJ0 commented Oct 15, 2022

zaptrem commented Dec 26, 2022

sampsyo left a comment

Choose a reason for hiding this comment

sampsyo Dec 29, 2022

Choose a reason for hiding this comment

JOJ0 Dec 29, 2022 • edited Loading

Choose a reason for hiding this comment

JOJ0 Dec 29, 2022 • edited Loading

Choose a reason for hiding this comment

JOJ0 commented Jan 2, 2023

sampsyo left a comment

Choose a reason for hiding this comment

sampsyo Jan 2, 2023

Choose a reason for hiding this comment

JOJ0 Jan 3, 2023

Choose a reason for hiding this comment

sampsyo Jan 3, 2023

Choose a reason for hiding this comment

sampsyo Jan 2, 2023

Choose a reason for hiding this comment

JOJ0 commented Jan 3, 2023

sampsyo commented Jan 6, 2023

Handle "join keyword" in `MetadataSourcePlugin.get_artist()` #4515

Handle "join keyword" in `MetadataSourcePlugin.get_artist()` #4515

JOJ0 commented Oct 10, 2022 •

edited

Loading

JOJ0 Dec 29, 2022 •

edited

Loading

JOJ0 Dec 29, 2022 •

edited

Loading