Merge branch 'master' into edit

beets/dbcore/sort.py

@@ -111,9 +111,8 @@ class FieldSort(Sort):
         self.case_insensitive = case_insensitive
 
     def sort(self, objs: list[AnyModel]) -> list[AnyModel]:
-        # TODO: Conversion and null-detection here. In Python 3,
-        # comparisons with None fail. We should also support flexible
-        # attributes with different types without falling over.
+        # TODO: Support flexible attributes with different types (e.g. a mix
+        # of strings and numbers) without falling over.
 
         def key(obj: Model) -> Any:
             field_val = obj.get(self.field, None)
@@ -126,7 +125,13 @@ class FieldSort(Sort):
                     field_val = ""
             if self.case_insensitive and isinstance(field_val, str):
                 field_val = field_val.lower()
-            return field_val
+            # Nullable types (e.g. ``NullInteger``/``NullFloat``) use ``None``
+            # as their null value, so a field may be missing on some objects
+            # and present on others. Comparing ``None`` with a real value
+            # raises a ``TypeError``, so group all missing values together:
+            # this places them first when sorting ascending and last when
+            # descending, matching SQLite's default ordering of NULLs.
+            return (field_val is not None, field_val)
 
         return sorted(objs, key=key, reverse=not self.ascending)
 

beets/test/helper.py

@@ -95,7 +95,7 @@ NEEDS_REFLINK = pytest.mark.skipif(
     not check_reflink_support(gettempdir()), reason="need reflink"
 )
 NEEDS_FFPROBE = pytest.mark.skipif(
-    not has_program("ffprobe") and not RUNNING_IN_CI,
+    not has_program("ffprobe", ("-version",)) and not RUNNING_IN_CI,
     reason="ffprobe (ffmpeg) is not available",
 )
 

docs/changelog.rst

@@ -59,6 +59,11 @@ Bug fixes
   instrumental track, and store that state in ``lyrics_instrumental`` flexible
   attribute. Existing ``[Instrumental]`` lyrics are migrated automatically.
   :bug:`6719`
+- Sorting by a nullable field (for example a flexible attribute with a declared
+  type whose null value is ``None``) that is present on only some items no
+  longer crashes with a ``TypeError``. Missing values are now grouped together,
+  ordered before present ones when sorting ascending and after them when
+  descending. :bug:`3461`
 
 ..
     For plugin developers

test/dbcore/test_sort.py

@@ -253,6 +253,34 @@ class TestNonExistingField:
         assert ids_asc == [*null_values_ids, lower_item.id, higher_item.id]
         assert ids_desc == [higher_item.id, lower_item.id, *null_values_ids]
 
+    @pytest.mark.parametrize(
+        "field_type,lower,higher",
+        [
+            _p(types.NullInteger(), 2, 10, id="null-integer"),
+            _p(types.NullFloat(), 2.5, 10.5, id="null-float"),
+        ],
+    )
+    def test_nullable_field_present_in_some_items(
+        self, monkeypatch, field_type, lower, higher
+    ):
+        """Ordering by a nullable-typed field (whose null value is ``None``)
+        that is present on only some items should not raise. See #3461.
+        """
+        monkeypatch.setitem(Item._types, "mynull", field_type)
+
+        lower_item, higher_item, *items_without_val = self.lib.items("id+")
+        for item, value in zip((lower_item, higher_item), (lower, higher)):
+            item.mynull = value
+            item.store()
+
+        null_values_ids = [i.id for i in items_without_val]
+
+        ids_asc = [i.id for i in self.lib.items("mynull+ id+")]
+        ids_desc = [i.id for i in self.lib.items("mynull- id+")]
+
+        assert ids_asc == [*null_values_ids, lower_item.id, higher_item.id]
+        assert ids_desc == [higher_item.id, lower_item.id, *null_values_ids]
+
     def test_negation_interaction(self):
         """Test the handling of negation and sorting together.
 

test/plugins/test_art.py

@@ -10,8 +10,6 @@ from unittest.mock import patch
 
 import confuse
 import pytest
-from requests_mock import ANY as ANYREEQUEST
-from requests_mock.exceptions import NoMockAddress
 
 from beets import config, importer, logging, util
 from beets.autotag import AlbumInfo, AlbumMatch, Distance
@@ -79,8 +77,8 @@ class UseThePlugin(TestHelper):
         # Register some “safe” mocks you actually want to allow, if any:
         # requests_mock.get("https://example.com/health", json={"status": "ok"})
 
-        # Optional: disable any URL not explicitly mocked
-        requests_mock.register_uri(ANYREEQUEST, ANYREEQUEST, exc=NoMockAddress)
+        # Disable any URL not explicitly mocked
+        return
 
     @pytest.fixture(autouse=True, scope="class")
     def cleanup(self):
@@ -343,6 +341,7 @@ class TestCombined(UseThePlugin, FetchImageHelper, CAAData):
     ASIN = "xxxx"
     MBID = "releaseid"
     AMAZON_URL = f"https://images.amazon.com/images/P/{ASIN}.01.LZZZZZZZ.jpg"
+    AMAZON_URL_2 = f"https://images.amazon.com/images/P/{ASIN}.02.LZZZZZZZ.jpg"
     AAO_URL = f"https://www.albumart.org/index_detail.php?asin={ASIN}"
 
     @pytest.fixture
@@ -394,6 +393,7 @@ class TestCombined(UseThePlugin, FetchImageHelper, CAAData):
 
     def test_main_interface_falls_back_to_aao(self, dpath, image_request_mock):
         image_request_mock.get(self.AMAZON_URL, content_type="text/html")
+        image_request_mock.get(self.AMAZON_URL_2, content_type="text/html")
         image_request_mock.get(self.AAO_URL, content_type="image/jpeg")
         album = Album(asin=self.ASIN)
         self.plugin.art_for_album(album, [dpath])