Fix unit test for spatial overlaps (DM-44362)

Using non-common skypix is going to be deprecated in favor of a
direct use of regions. New query system raises NotImplementedError
for non-common skypix, this patch replaces that exception with
a new exception class to support exception propagation from remote
butler to client.

`test_skypix_constraint_queries` unit test is updated to handle
exceptions from the new query system. When direct butler switches
to the new query system, that test can be removed or changed to
always expect the exceptions.

python/lsst/daf/butler/_exceptions.py

@@ -155,6 +155,14 @@ class MissingCollectionError(CollectionError, ButlerUserError):
     error_type = "missing_collection"
 
 
+class UnimplementedQueryError(NotImplementedError, ButlerUserError):
+    """Exception raised when the query system does not support the query
+    specified by the user.
+    """
+
+    error_type = "unimplemented_query"
+
+
 class MissingDatasetTypeError(DatasetTypeError, KeyError, ButlerUserError):
     """Exception raised when a dataset type does not exist."""
 
@@ -216,6 +224,7 @@ class UnknownButlerUserError(ButlerUserError):
     InvalidQueryError,
     MissingCollectionError,
     MissingDatasetTypeError,
+    UnimplementedQueryError,
     UnknownButlerUserError,
 )
 _USER_ERROR_MAPPING = {e.error_type: e for e in _USER_ERROR_TYPES}

python/lsst/daf/butler/direct_query_driver/_driver.py

@@ -484,7 +484,7 @@ def analyze_query(
             Column expressions to sort by.
         find_first_dataset : `str` or `None`, optional
             Name of a dataset type for which only one result row for each data
-            ID should be returned, with the colletions searched in order.
+            ID should be returned, with the collections searched in order.
 
         Returns
         -------

python/lsst/daf/butler/registry/dimensions/static.py

@@ -40,6 +40,7 @@
 from ... import ddl
 from ..._column_tags import DimensionKeyColumnTag, DimensionRecordColumnTag
 from ..._column_type_info import LogicalColumn
+from ..._exceptions import UnimplementedQueryError
 from ..._named import NamedKeyDict
 from ...dimensions import (
     DatabaseDimensionElement,
@@ -1100,7 +1101,7 @@ def visit_spatial_join(
         # Reject spatial joins that are nested inside OR or NOT, because the
         # postprocessing needed for those would be a lot harder.
         if flags & PredicateVisitFlags.INVERTED or flags & PredicateVisitFlags.HAS_OR_SIBLINGS:
-            raise NotImplementedError("Spatial overlap joins nested inside OR or NOT are not supported.")
+            raise UnimplementedQueryError("Spatial overlap joins nested inside OR or NOT are not supported.")
         # Delegate to super to check for invalid joins and record this
         # "connection" for use when seeing whether to add an automatic join
         # later.
@@ -1137,7 +1138,7 @@ def visit_spatial_join(
                 # tile is necessary but not sufficient for that.
                 self.builder.postprocessing.spatial_join_filtering.append((a, b))
             case _:
-                raise NotImplementedError(f"Unsupported combination for spatial join: {a, b}.")
+                raise UnimplementedQueryError(f"Unsupported combination for spatial join: {a, b}.")
         return qt.Predicate.from_bool(True)
 
     def _join_common_skypix_overlap(self, element: DatabaseDimensionElement) -> None:

python/lsst/daf/butler/registry/tests/_registry.py

@@ -30,6 +30,7 @@
 
 __all__ = ["RegistryTests"]
 
+import contextlib
 import datetime
 import itertools
 import os
@@ -3644,27 +3645,38 @@ def test_skypix_constraint_queries(self) -> None:
                 break
         else:
             raise RuntimeError("Could not find usable skypix ID for this dimension configuration.")
-        self.assertEqual(
-            {
-                (data_id["tract"], data_id["patch"])
-                for data_id in registry.queryDataIds(
-                    ["patch"],
-                    dataId={skypix_dimension.name: skypix_id},
-                )
-            },
-            overlapping_patches,
-        )
+        # New query system does not support non-common skypix constraints
+        # and we are deprecating it to replace with region-based constrints.
+        # TODO: Drop this tests once we remove support for non-common skypix.
+        with contextlib.suppress(NotImplementedError):
+            self.assertEqual(
+                {
+                    (data_id["tract"], data_id["patch"])
+                    for data_id in registry.queryDataIds(
+                        ["patch"],
+                        dataId={skypix_dimension.name: skypix_id},
+                    )
+                },
+                overlapping_patches,
+            )
         # Test that a three-way join that includes the common skypix system in
         # the dimensions doesn't generate redundant join terms in the query.
-        full_data_ids = set(
-            registry.queryDataIds(
-                ["tract", "visit", "htm7"], skymap="hsc_rings_v1", instrument="HSC"
-            ).expanded()
-        )
-        self.assertGreater(len(full_data_ids), 0)
-        for data_id in full_data_ids:
-            self.assertFalse(data_id.records["tract"].region.isDisjointFrom(data_id.records["htm7"].region))
-            self.assertFalse(data_id.records["visit"].region.isDisjointFrom(data_id.records["htm7"].region))
+        # TODO: In the new query system this raises InvalidQueryError, change
+        # the test to assertRaises when direct butler switches to new queries.
+        with contextlib.suppress(InvalidQueryError):
+            full_data_ids = set(
+                registry.queryDataIds(
+                    ["tract", "visit", "htm7"], skymap="hsc_rings_v1", instrument="HSC"
+                ).expanded()
+            )
+            self.assertGreater(len(full_data_ids), 0)
+            for data_id in full_data_ids:
+                self.assertFalse(
+                    data_id.records["tract"].region.isDisjointFrom(data_id.records["htm7"].region)
+                )
+                self.assertFalse(
+                    data_id.records["visit"].region.isDisjointFrom(data_id.records["htm7"].region)
+                )
 
     def test_spatial_constraint_queries(self) -> None:
         """Test queries in which one spatial dimension in the constraint (data

tests/test_remote_butler.py

@@ -199,11 +199,6 @@ def test_query_projection_drop_postprocessing(self):
         # RemoteButler.
         pass
 
-    @unittest.expectedFailure
-    def test_skypix_constraint_queries(self) -> None:
-        # TODO DM-44362
-        return super().test_skypix_constraint_queries()
-
 
 @unittest.skipIf(create_test_server is None, "Server dependencies not installed.")
 class RemoteButlerSqliteRegistryTests(RemoteButlerRegistryTests, unittest.TestCase):