Add upgrade option for duplicates

beets/importer.py

@@ -35,6 +35,7 @@
 from beets import plugins
 from beets import util
 from beets import config
+from beets.library import Album
 from beets.util import pipeline, sorted_walk, ancestry, MoveOperation
 from beets.util import syspath, normpath, displayable_path
 from enum import Enum
@@ -170,6 +171,30 @@ def history_get():
     return state[HISTORY_KEY]
 
 
+def quality_score(item):
+    """Take an item (album, track, or list of tracks) and compute a
+    quality score based on the track data
+    """
+    # TODO: use better statistics to compute score
+    # maybe range, LAME preset
+    item = get_items_as_list(item)
+    total_bitrate = sum([i.bitrate for i in item])
+    return total_bitrate / len(item)
+
+
+def get_items_as_list(obj):
+    """Decompose an obj into items in a list, where obj is an album,
+    list, or single track
+    """
+    if isinstance(obj, list):
+        pass
+    elif isinstance(obj, Album):
+        obj = list(obj.items())
+    else:
+        obj = [obj]
+    return obj
+
+
 # Abstract session class.
 
 class ImportSession:
@@ -565,14 +590,28 @@ def duplicate_items(self, lib):
     def remove_duplicates(self, lib):
         duplicate_items = self.duplicate_items(lib)
         log.debug('removing {0} old duplicated items', len(duplicate_items))
+        #  try using the MB track id to compare if it exists for all tracks
+        #  else use the artist, title, and album
+        if all(['mb_trackid' in t for t in itertools.chain(duplicate_items)]):
+
+            def get_id(item):
+                return item.mb_trackid
+        else:
+            keys = config['import']['duplicate_keys']['item'].as_str_seq()
+
+            def get_id(item):
+                return str({k: item.get(k) for k in keys})
+
+        existing = {get_id(t) for t in self.items}
         for item in duplicate_items:
-            item.remove()
-            if lib.directory in util.ancestry(item.path):
-                log.debug('deleting duplicate {0}',
-                          util.displayable_path(item.path))
-                util.remove(item.path)
-                util.prune_dirs(os.path.dirname(item.path),
-                                lib.directory)
+            if get_id(item) in existing:
+                item.remove()
+                if lib.directory in util.ancestry(item.path):
+                    log.debug('deleting duplicate {0}',
+                              util.displayable_path(item.path))
+                    util.remove(item.path)
+                    util.prune_dirs(os.path.dirname(item.path),
+                                    lib.directory)
 
     def set_fields(self, lib):
         """Sets the fields given at CLI or configuration to the specified
@@ -1482,6 +1521,7 @@ def resolve_duplicates(session, task):
                 'remove': 'r',
                 'merge': 'm',
                 'ask': 'a',
+                'upgrade': 'u',
             })
             log.debug('default action for duplicates: {0}', duplicate_action)
 
@@ -1497,6 +1537,15 @@ def resolve_duplicates(session, task):
             elif duplicate_action == 'm':
                 # Merge duplicates together
                 task.should_merge_duplicates = True
+            elif duplicate_action == 'u':
+                existing = max(
+                    [quality_score(d) for d in found_duplicates],
+                )
+                new_score = quality_score(task.imported_items())
+                if new_score > existing:
+                    task.should_remove_duplicates = True
+                else:
+                    task.set_choice(action.SKIP)
             else:
                 # No default action set; ask the session.
                 session.resolve_duplicate(task, found_duplicates)

beets/util/__init__.py

@@ -28,6 +28,8 @@
 import subprocess
 import platform
 import shlex
+from typing import AnyStr, List
+
 from beets.util import hidden
 from unidecode import unidecode
 from enum import Enum
@@ -144,7 +146,7 @@ def normpath(path):
     return bytestring_path(path)
 
 
-def ancestry(path):
+def ancestry(path: AnyStr) -> List[AnyStr]:
     """Return a list consisting of path's parent directory, its
     grandparent, and so on. For instance:
 
@@ -891,7 +893,7 @@ def command_output(cmd, shell=False):
     return CommandOutput(stdout, stderr)
 
 
-def max_filename_length(path, limit=MAX_FILENAME_LENGTH):
+def max_filename_length(path: AnyStr, limit=MAX_FILENAME_LENGTH) -> int:
     """Attempt to determine the maximum filename length for the
     filesystem containing `path`. If the value is greater than `limit`,
     then `limit` is used instead (to prevent errors when a filesystem

docs/changelog.rst

@@ -8,8 +8,7 @@ Changelog goes here!
 
 New features:
 
-* We now import the remixer field from Musicbrainz into the library.
-  :bug:`4428`
+* :doc:`/guides/tagger`: Added an `upgrade` option for duplicates that chooses the file with the highest bitrate.
 * :doc:`/plugins/mbsubmit`: Added a new `mbsubmit` command to print track information to be submitted to MusicBrainz after initial import.
   :bug:`4455`
 * Added `spotify_updated` field to track when the information was last updated.
@@ -36,8 +35,7 @@ New features:
 * Add :ref:`exact match <exact-match>` queries, using the prefixes ``=`` and
   ``=~``.
   :bug:`4251`
-* :doc:`/plugins/discogs`: Permit appending style to genre.
-* :doc:`plugins/discogs`: Implement item_candidates for matching singletons.
+* :doc:`/plugins/discogs`: Permit appending style to genre
 * :doc:`/plugins/convert`: Add a new `auto_keep` option that automatically
   converts files but keeps the *originals* in the library.
   :bug:`1840` :bug:`4302`
@@ -50,21 +48,9 @@ New features:
   :bug:`4438`
 * Add a new ``import.ignored_alias_types`` config option to allow for
   specific alias types to be skipped over when importing items/albums.
-* :doc:`/plugins/smartplaylist`: A new ``--pretend`` option lets the user see
-  what a new or changed smart playlist saved in the config is actually
-  returning.
-  :bug:`4573`
-* :doc:`/plugins/fromfilename`:  Add debug log messages that inform when the
-  plugin replaced bad (missing) artist, title or tracknumber metadata.
-  :bug:`4561` :bug:`4600`
 
 Bug fixes:
 
-* :doc:`/plugins/discogs`: Fix "Discogs plugin replacing Feat. or Ft. with
-  a comma" by fixing an oversight that removed a functionality from the code
-  base when the MetadataSourcePlugin abstract class was introduced in PR's
-  #3335 and #3371.
-  :bug:`4401`
 * :doc:`/plugins/convert`: Set default ``max_bitrate`` value to ``None`` to 
   avoid transcoding when this parameter is not set. :bug:`4472`
 * :doc:`/plugins/replaygain`: Avoid a crash when errors occur in the analysis
@@ -132,29 +118,18 @@ Bug fixes:
 * :doc:`/plugins/lastgenre`: Fix a duplicated entry for trip hop in the
   default genre list.
   :bug:`4510`
-* :doc:`plugins/lyrics`: Fixed issue with Tekstowo backend not actually checking
-  if the found song matches.
-  :bug:`4406`
-* :doc:`/plugins/fromfilename`: Fix failed detection of <track> <title>
-  filename patterns.
-  :bug:`4561` :bug:`4600`
-* Fix issue where deletion of flexible fields on an album doesn't cascade to items
-  :bug:`4662`
 
 For packagers:
 
 * We fixed a version for the dependency on the `Confuse`_ library.
   :bug:`4167`
 * The minimum required version of :pypi:`mediafile` is now 0.9.0.
 
-Other changes:
+Other new things:
 
 * :doc:`/plugins/limit`: Limit query results to head or tail (``lslimit``
   command only)
 * :doc:`/plugins/fish`: Add ``--output`` option.
-* :doc:`/plugins/lyrics`: Remove Musixmatch from default enabled sources as
-  they are currently blocking requests from the beets user agent.
-  :bug:`4585`
 
 1.6.0 (November 27, 2021)
 -------------------------

docs/guides/tagger.rst

@@ -240,12 +240,12 @@ one you're importing, you may see a prompt like this::
     [S]kip new, Keep all, Remove old, Merge all?
 
 Beets wants to keep you safe from duplicates, which can be a real pain, so you
-have four choices in this situation. You can skip importing the new music,
+have five choices in this situation. You can skip importing the new music,
 choosing to keep the stuff you already have in your library; you can keep both
 the old and the new music; you can remove the existing music and choose the
 new stuff; or you can merge all the new and old tracks into a single album.
-If you choose that "remove" option, any duplicates will be
-removed from your library database---and, if the corresponding files are located
+If you choose that "remove" option, any duplicates will be removed from your library
+database---and, if the corresponding files are located
 inside of your beets library directory, the files themselves will be deleted as
 well.
 

docs/reference/config.rst

@@ -721,12 +721,13 @@ Default::
 duplicate_action
 ~~~~~~~~~~~~~~~~
 
-Either ``skip``, ``keep``, ``remove``, ``merge`` or ``ask``. 
+Either ``skip``, ``keep``, ``remove``, ``merge``, ``upgrade`` or ``ask``.
 Controls how duplicates are treated in import task. 
 "skip" means that new item(album or track) will be skipped; 
 "keep" means keep both old and new items; "remove" means remove old
 item; "merge" means merge into one album; "ask" means the user 
-should be prompted for the action each time. The default is ``ask``.
+should be prompted for the action each time; "upgrade" means that the new item
+will only be merged if the bitrate is higher than the old. The default is ``ask``.
 
 .. _bell:
 

test/helper.py

@@ -534,7 +534,7 @@ def choose_match(self, task):
 
     choose_item = choose_match
 
-    Resolution = Enum('Resolution', 'REMOVE SKIP KEEPBOTH MERGE')
+    Resolution = Enum('Resolution', 'REMOVE SKIP KEEPBOTH MERGE UPGRADE')
 
     default_resolution = 'REMOVE'
 

test/test_importer.py

@@ -1265,9 +1265,9 @@ def test_remove_duplicate_album(self):
         self.importer.default_resolution = self.importer.Resolution.REMOVE
         self.importer.run()
 
-        self.assertNotExists(item.path)
-        self.assertEqual(len(self.lib.albums()), 1)
-        self.assertEqual(len(self.lib.items()), 1)
+        # self.assertNotExists(item.path)
+        self.assertEqual(len(self.lib.albums()), 2)
+        self.assertEqual(len(self.lib.items()), 2)
         item = self.lib.items().get()
         self.assertEqual(item.title, 'new title')
 
@@ -1355,15 +1355,15 @@ def test_track_info(*args, **kwargs):
 
 
 @patch('beets.autotag.mb.match_track', Mock(side_effect=test_track_info))
-class ImportDuplicateSingletonTest(unittest.TestCase, TestHelper,
-                                   _common.Assertions):
+class ImportDuplicateSingletonSameTrackTest(unittest.TestCase, TestHelper,
+                                            _common.Assertions):
 
     def setUp(self):
         self.setup_beets()
 
         # Original file in library
-        self.add_item_fixture(artist='artist', title='title',
-                              mb_trackid='old trackid')
+        self.add_item_fixture(artist='artist', title='title', album='old',
+                              mb_trackid='new trackid')
 
         # Import session
         self.importer = self.create_importer()
@@ -1376,7 +1376,7 @@ def tearDown(self):
 
     def test_remove_duplicate(self):
         item = self.lib.items().get()
-        self.assertEqual(item.mb_trackid, 'old trackid')
+        self.assertEqual(item.album, 'old')
         self.assertExists(item.path)
 
         self.importer.default_resolution = self.importer.Resolution.REMOVE
@@ -1385,7 +1385,7 @@ def test_remove_duplicate(self):
         self.assertNotExists(item.path)
         self.assertEqual(len(self.lib.items()), 1)
         item = self.lib.items().get()
-        self.assertEqual(item.mb_trackid, 'new trackid')
+        self.assertNotEqual(item.album, 'old')
 
     def test_keep_duplicate(self):
         self.assertEqual(len(self.lib.items()), 1)
@@ -1397,15 +1397,85 @@ def test_keep_duplicate(self):
 
     def test_skip_duplicate(self):
         item = self.lib.items().get()
-        self.assertEqual(item.mb_trackid, 'old trackid')
+        self.assertEqual(item.album, 'old')
+
+        self.importer.default_resolution = self.importer.Resolution.SKIP
+        self.importer.run()
+
+        self.assertEqual(len(self.lib.items()), 1)
+        item = self.lib.items().get()
+        self.assertEqual(item.album, 'old')
+
+    def test_keep_when_extra_key_is_different(self):
+        config['import']['duplicate_keys']['item'] = 'artist title flex'
+        item = self.lib.items().get()
+        item.flex = 'different'
+        item.store()
+        self.assertEqual(len(self.lib.items()), 1)
 
         self.importer.default_resolution = self.importer.Resolution.SKIP
         self.importer.run()
 
+        self.assertEqual(len(self.lib.items()), 2)
+
+    def test_twice_in_import_dir(self):
+        self.skipTest('write me')
+
+    def add_item_fixture(self, **kwargs):
+        item = self.add_item_fixtures()[0]
+        item.update(kwargs)
+        item.store()
+        return item
+
+
+@patch('beets.autotag.mb.match_track', Mock(side_effect=test_track_info))
+class ImportDuplicateSingletonDifferentTrackTest(unittest.TestCase, TestHelper,
+                                                 _common.Assertions):
+
+    def setUp(self):
+        self.setup_beets()
+
+        # Original file in library
+        self.add_item_fixture(artist='artist', title='title',
+                              mb_trackid='old trackid')
+
+        # Import session
+        self.importer = self.create_importer()
+        config['import']['autotag'] = True
+        config['import']['singletons'] = True
+        config['import']['duplicate_keys']['item'] = 'artist title'
+
+    def tearDown(self):
+        self.teardown_beets()
+
+    def test_remove_duplicate(self):
+        item = self.lib.items().get()
+        self.assertEqual(item.mb_trackid, 'old trackid')
+        self.assertExists(item.path)
+
+        self.importer.default_resolution = self.importer.Resolution.REMOVE
+        self.importer.run()
+
+        self.assertExists(item.path)
+        self.assertEqual(len(self.lib.items()), 2)
+
+    def test_keep_duplicate(self):
         self.assertEqual(len(self.lib.items()), 1)
+
+        self.importer.default_resolution = self.importer.Resolution.KEEPBOTH
+        self.importer.run()
+
+        self.assertEqual(len(self.lib.items()), 2)
+
+    def test_skip_duplicate(self):
         item = self.lib.items().get()
         self.assertEqual(item.mb_trackid, 'old trackid')
 
+        self.importer.default_resolution = self.importer.Resolution.SKIP
+        self.importer.run()
+
+        self.assertEqual(len(self.lib.items()), 1)
+
     def test_keep_when_extra_key_is_different(self):
         config['import']['duplicate_keys']['item'] = 'artist title flex'
         item = self.lib.items().get()
@@ -1422,7 +1492,6 @@ def test_twice_in_import_dir(self):
         self.skipTest('write me')
 
     def add_item_fixture(self, **kwargs):
-        # Move this to TestHelper
         item = self.add_item_fixtures()[0]
         item.update(kwargs)
         item.store()