Support providing album information on singleton imports via Discogs #4854

JOJ0 · 2023-07-18T10:56:51Z

Description

The Discogs metadata plugin now saves album information when importing singletons.

Fixes discogs: Save album information on singleton imports #4716
Is a refactored / simplified version of OLD Support providing album information on singleton imports #4717
Presents album information in the singleton importer UI as:

# selection (default 1), Skip, Use as-is, Enter search, enter Id, aBort,
eDit, edit Candidates? 4
Tagging track: Portishead - Numb
               Album: Numb
URL:
    https://www.discogs.com/release/1385011-Portishead-Numb
(Similarity: 100.0%) (Discogs, Index 1, Track A1)
Apply, More candidates, Skip, Use as-is, Enter search, enter Id, aBort,
eDit, edit Candidates?

Additionally this PR extends the "disambiguation string" for all singleton imports with track index and alternative name (track_alt field):

Candidates:
1. Portishead - Numb (100.0%) (Spotify, Index 7)
2. Portishead - Numb (100.0%) (Spotify, Index 1)
3. Portishead - Numb (100.0%) (Spotify, Index 27)
4. Portishead - Numb (100.0%) (Discogs, Index 1, Track A1)
5. Portishead - Numb (100.0%) (Discogs, Index 1, Track A1)
6. Portishead - Numb (100.0%) (Discogs, Index 1, Track 1)
7. Portishead - Numb (100.0%) (Discogs, Index 1, Track 1)
8. Portishead - Numb (100.0%) (Discogs, Index 1, Track 1)

Furthermore if an album name is found, it's appended to the end of the disambiguation string. This is currently supported by the Discogs and the Spotify plugin:

Candidates:
1. Portishead - Numb (100.0%) (Discogs, Index 1, Track A1, [Numb])
2. Portishead - Numb (100.0%) (Discogs, Index 1, Track A1, [Numb])
3. Portishead - Numb (100.0%) (Discogs, Index 1, Track 1, [Numb])
4. Portishead - Numb (100.0%) (Discogs, Index 1, Track 1, [Numb])
5. Portishead - Numb (100.0%) (Discogs, Index 1, Track 1, [Numb])
6. Portishead - Numb (100.0%) (Spotify, Index 7, [Dummy])
7. Portishead - Numb (100.0%) (Spotify, Index 1, [Numb])
8. Portishead - Numb (100.0%) (Spotify, Index 27, [Crazy Mix 5])
9. Portishead - Numbed In Moscow (64.3%) (title) (Spotify, Index 2, [Numb])

This feature is enabled by default but if such a long disambiguation string is not desired by the user, displaying of [Album] can be turned off using a new configuration option:

import:
    singleton_album_disambig: no

To Do

Documentation.
Changelog.
~~Tests.~~

In DiscogsPlugin.get_albums() we already strip away the words "CD" and "disk". It makes sense to also remove "vinyl"

PR beetbox#4854

JOJ0 · 2023-07-18T11:44:00Z

Hi @sampsyo thanks again for the suggestion for simplification of this feature's code: #4717 (comment)

I finally managed to find time to refactor it and give your simplification idea a try. For the sake of comprehensibility I left the old code/PR in-tact and created this new and simplified one. Unfortunately applying a whole AlbumInfo object to the candidate TrackInfo object resulted in the following array while applying:

/home/jojo/Music/Portishead/[2000] Undercover Themes/09 - Earth Linger.flac
Correcting track tags from:
    Portishead - Earth Linger
To:
    Portishead - Numb (Earth - Linger)
URL:
    https://www.discogs.com/release/1385011-Portishead-Numb
(Similarity: 81.0%) (title) (Discogs)
[A]pply, More candidates, Skip, Use as-is, Enter search, enter Id, aBort,
eDit, edit Candidates? 
Traceback (most recent call last):
  File "/home/jojo/.pyenv/versions/beets/bin/beet", line 33, in <module>
    sys.exit(load_entry_point('beets', 'console_scripts', 'beet')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jojo/git/beets/beets/ui/__init__.py", line 1301, in main
    _raw_main(args)
  File "/home/jojo/git/beets/beets/ui/__init__.py", line 1288, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/home/jojo/git/beets/beets/ui/commands.py", line 1037, in import_func
    import_files(lib, paths, query)
  File "/home/jojo/git/beets/beets/ui/commands.py", line 977, in import_files
    session.run()
  File "/home/jojo/git/beets/beets/importer.py", line 353, in run
    pl.run_parallel(QUEUE_SIZE)
  File "/home/jojo/git/beets/beets/util/pipeline.py", line 446, in run_parallel
    raise exc_info[1].with_traceback(exc_info[2])
  File "/home/jojo/git/beets/beets/util/pipeline.py", line 311, in run
    out = self.coro.send(msg)
          ^^^^^^^^^^^^^^^^^^^
  File "/home/jojo/git/beets/beets/util/pipeline.py", line 170, in coro
    task = func(*(args + (task,)))
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jojo/git/beets/beets/importer.py", line 1512, in user_query
    apply_choice(session, task)
  File "/home/jojo/git/beets/beets/importer.py", line 1583, in apply_choice
    task.add(session.lib)
  File "/home/jojo/git/beets/beets/importer.py", line 995, in add
    lib.add(self.item)
  File "/home/jojo/git/beets/beets/library.py", line 1504, in add
    obj.add(self)
  File "/home/jojo/git/beets/beets/library.py", line 374, in add
    super().add(lib)
  File "/home/jojo/git/beets/beets/dbcore/db.py", line 628, in add
    self.store()
  File "/home/jojo/git/beets/beets/library.py", line 366, in store
    super().store(fields)
  File "/home/jojo/git/beets/beets/dbcore/db.py", line 557, in store
    tx.mutate(
  File "/home/jojo/git/beets/beets/dbcore/db.py", line 937, in mutate
    cursor = self.db._connection().execute(statement, subvals)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.ProgrammingError: Error binding parameter 3: type 'AlbumInfo' is not supported

Without having investigated too much, it seemed to me that the TrackInfo objects generated during this early stage in the import process are passed all the way through to the database abstraction layer. where it finally wants to store this unsupported field. Removing it on this level again felt unnecessary complicated so I decided to go down an even simpler route of just applying, what I actually need, to the TrackInfo object: The album name. This is actually the way the Spotify plugin does it too.

What do you think about this solution?

Also: Please a wording check of my additions to the docs: 1ab8ae8

PR beetbox#4854

beets/ui/commands.py

wisp3rwind · 2023-07-18T19:23:31Z

Without having investigated too much, it seemed to me that the TrackInfo objects generated during this early stage in the import process are passed all the way through to the database abstraction layer. where it finally wants to store this unsupported field. Removing it on this level again felt unnecessary complicated so I decided to go down an even simpler route of just applying, what I actually need, to the TrackInfo object: The album name. This is actually the way the Spotify plugin does it too.

Yes, looks like it's trying to assign the AlbumInfo to an item field, which makes sense since you just set it as any other TrackInfo field.

What do you think about this solution?

Looks alright to me, we can still come up with a more elaborate solution should we ever need more album fields on TrackInfo.

JOJ0 · 2023-07-19T06:22:42Z

What do you think about this solution?

Looks alright to me, we can still come up with a more elaborate solution should we ever need more album fields on TrackInfo.

Perfect!

Thanks so much for the review!

When available, display e.g: - Track Index as "Index 2" - Alternative Track name as "Track A2"

to the TrackInfo objects it returns. Additionally a new feature is introduced that uses string_dist to find the correct track on the Discogs album.

- If the file being imported has an album tag already, display it. - If the metadata plugin provides a new album value, preview the change.

PR beetbox#4854

conditions. Co-authored-by: Benedikt <[email protected]>

- New config option for the importer 'singleton_album_disambig' lets users choose whether they want to display [album names] in the list of candidates. Enabled by default but ony applicable if the candidate provides an album attribute. - Add docs describing the new option and which source plugins currently support it.

JOJ0 · 2023-07-21T06:35:04Z

Alright, I think I'm happy with this feature now. In the end I decided to additionally add the album name to the disambiguation string, which makes the selection of the "correct" release even more straight forward. Also I realized that it makes sense to make this globally configurable since the Spotify plugin will benefit from it as well and some users might find such a long disambiguation string distracting. See my additions to the inital PR description.

I tried to explain as brief and clear as possible but nitpicking on my docs wording is always very welcome :-)

a374977#diff-6d7bb6bc84b63dd7d8b09e64b4d5b117575f689ecb5a492c25b9aeeff1b5b38fR765-R774

wisp3rwind

LGTM; however, I've only looked at the code changes in isolation, since I don't know the discogs plugin (in fact, any of the metadata sources) in detail.

JOJ0 · 2023-07-25T04:34:07Z

Thanks @wisp3rwind, that's perfectly fine and helpful. Thanks for checking through! I'm happy to finally have merged this feature. I had been successfully using its first version for quite some time now, which shares parts of the Discogs plugin code we have here and I'm also very happy that now it's so much simpler than it was in the first place. (thanks again @sampsyo for the hints to simplify ;-)))

PR beetbox#4854

JOJ0 added 2 commits July 18, 2023 07:51

Fix early exit in Discogs item_candidates method

3debf4e

Strip word "vinyl" when searching Discogs

0630f4a

In DiscogsPlugin.get_albums() we already strip away the words "CD" and "disk". It makes sense to also remove "vinyl"

JOJ0 added a commit to JOJ0/beets that referenced this pull request Jul 18, 2023

Add changelog for beetbox#4854

61cec3c

JOJ0 added a commit to JOJ0/beets that referenced this pull request Jul 18, 2023

Add singleton matching info to Discogs plugin docs

1ab8ae8

PR beetbox#4854

JOJ0 added a commit to JOJ0/beets that referenced this pull request Jul 18, 2023

Add singleton matching info to Discogs plugin docs

70b9d22

PR beetbox#4854

JOJ0 force-pushed the discogs_single_album_refactor branch from 1ab8ae8 to 70b9d22 Compare July 18, 2023 11:51

wisp3rwind reviewed Jul 18, 2023

View reviewed changes

beets/ui/commands.py Outdated Show resolved Hide resolved

JOJ0 force-pushed the discogs_single_album_refactor branch 2 times, most recently from c931c6c to a39ecff Compare July 19, 2023 06:18

JOJ0 and others added 7 commits July 20, 2023 09:12

Improve singleton disambiguation string

106a163

When available, display e.g: - Track Index as "Index 2" - Alternative Track name as "Track A2"

Add optional album field to candidate TrackInfo

090cfc6

DiscogsPlugin.item_candidates attaches album info

f84b9ea

to the TrackInfo objects it returns. Additionally a new feature is introduced that uses string_dist to find the correct track on the Discogs album.

Show album information during singleton imports

4272cbe

- If the file being imported has an album tag already, display it. - If the metadata plugin provides a new album value, preview the change.

Add changelog for beetbox#4854

d61b7c9

Add singleton matching info to Discogs plugin docs

1e050e2

PR beetbox#4854

Simplify singleton importer "Album: " print

4087e8c

conditions. Co-authored-by: Benedikt <[email protected]>

JOJ0 force-pushed the discogs_single_album_refactor branch 2 times, most recently from 0573088 to ac34557 Compare July 21, 2023 06:05

JOJ0 force-pushed the discogs_single_album_refactor branch from ac34557 to a374977 Compare July 21, 2023 06:33

JOJ0 requested review from wisp3rwind and sampsyo July 21, 2023 06:36

wisp3rwind approved these changes Jul 22, 2023

View reviewed changes

JOJ0 merged commit c03bd26 into beetbox:master Jul 25, 2023

JOJ0 mentioned this pull request Jul 25, 2023

Discogs: rank candidates before return in item_candidates #4703

Closed

JOJ0 mentioned this pull request Jul 31, 2023

Add customisable disambiguation strings for importer #4547

Merged

3 tasks

Louson pushed a commit to Louson/beets that referenced this pull request Sep 26, 2023

Add changelog for beetbox#4854

dd186e1

Louson pushed a commit to Louson/beets that referenced this pull request Sep 26, 2023

Add singleton matching info to Discogs plugin docs

ebf713e

PR beetbox#4854

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support providing album information on singleton imports via Discogs #4854

Support providing album information on singleton imports via Discogs #4854

JOJ0 commented Jul 18, 2023 •

edited

Loading

JOJ0 commented Jul 18, 2023 •

edited

Loading

wisp3rwind commented Jul 18, 2023

JOJ0 commented Jul 19, 2023

JOJ0 commented Jul 21, 2023

wisp3rwind left a comment

JOJ0 commented Jul 25, 2023 •

edited

Loading

Support providing album information on singleton imports via Discogs #4854

Support providing album information on singleton imports via Discogs #4854

Conversation

JOJ0 commented Jul 18, 2023 • edited Loading

Description

To Do

JOJ0 commented Jul 18, 2023 • edited Loading

wisp3rwind commented Jul 18, 2023

JOJ0 commented Jul 19, 2023

JOJ0 commented Jul 21, 2023

wisp3rwind left a comment

Choose a reason for hiding this comment

JOJ0 commented Jul 25, 2023 • edited Loading

JOJ0 commented Jul 18, 2023 •

edited

Loading

JOJ0 commented Jul 18, 2023 •

edited

Loading

JOJ0 commented Jul 25, 2023 •

edited

Loading