diff --git a/python/lsst/daf/butler/registry/queries/expressions/categorize.py b/python/lsst/daf/butler/registry/queries/expressions/categorize.py index f7a4d695a6..62144d357d 100644 --- a/python/lsst/daf/butler/registry/queries/expressions/categorize.py +++ b/python/lsst/daf/butler/registry/queries/expressions/categorize.py @@ -88,12 +88,17 @@ def categorizeElementId(universe: DimensionUniverse, name: str) -> tuple[Dimensi so at least its message should generally be propagated up to a context where the error can be interpreted by a human. """ - table, sep, column = name.partition(".") + table, _, column = name.partition(".") if column: try: element = universe[table] - except KeyError as err: - raise LookupError(f"No dimension element with name '{table}'.") from err + except KeyError: + if table == "timespan" or table == "datetime" or table == "timestamp": + raise LookupError( + "Dimension element name cannot be inferred in this context; " + f"use .timespan.{column} instead." + ) from None + raise LookupError(f"No dimension element with name {table!r} in {name!r}.") from None if isinstance(element, Dimension) and column == element.primaryKey.name: # Allow e.g. "visit.id = x" instead of just "visit = x"; this # can be clearer. @@ -172,8 +177,9 @@ def categorizeOrderByName(graph: DimensionGraph, name: str) -> tuple[DimensionEl field_name = name elif len(matches) > 1: raise ValueError( - f"Timespan exists in more than one dimesion element: {matches}," - " qualify timespan with specific dimension name." + "Timespan exists in more than one dimension element " + f"({', '.join(element.name for element in matches)}); " + "qualify timespan with specific dimension name." ) else: raise ValueError(f"Cannot find any temporal dimension element for '{name}'.") @@ -197,8 +203,9 @@ def categorizeOrderByName(graph: DimensionGraph, name: str) -> tuple[DimensionEl field_name = name elif len(match_pairs) > 1: raise ValueError( - f"Metadata '{name}' exists in more than one dimension element: " - f"{[element for element, _ in match_pairs]}, qualify metadata name with dimension name." + f"Metadata '{name}' exists in more than one dimension element " + f"({', '.join(element.name for element, _ in match_pairs)}); " + "qualify field name with dimension name." ) else: raise ValueError(f"Metadata '{name}' cannot be found in any dimension.") @@ -206,7 +213,11 @@ def categorizeOrderByName(graph: DimensionGraph, name: str) -> tuple[DimensionEl # qualified name, must be a dimension element and a field elem_name, _, field_name = name.partition(".") if elem_name not in graph.elements.names: - raise ValueError(f"Unknown dimension element name '{elem_name}'") + if field_name == "begin" or field_name == "end": + raise ValueError( + f"Unknown dimension element {elem_name!r}; perhaps you meant 'timespan.{field_name}'?" + ) + raise ValueError(f"Unknown dimension element {elem_name!r}.") element = graph.elements[elem_name] if field_name in ("timespan.begin", "timespan.end"): if not element.temporal: @@ -289,7 +300,11 @@ def categorizeElementOrderByName(element: DimensionElement, name: str) -> str | # qualified name, must be a dimension element and a field elem_name, _, field_name = name.partition(".") if elem_name != element.name: - raise ValueError(f"Element name mismatch: '{elem_name}' instead of '{element}'") + if field_name == "begin" or field_name == "end": + extra = f"; perhaps you meant 'timespan.{field_name}'?" + else: + extra = "." + raise ValueError(f"Element name mismatch: '{elem_name}' instead of '{element}'{extra}") if field_name in ("timespan.begin", "timespan.end"): if not element.temporal: raise ValueError(f"Cannot use '{field_name}' with non-temporal element '{element}'.") diff --git a/python/lsst/daf/butler/registry/tests/_registry.py b/python/lsst/daf/butler/registry/tests/_registry.py index 558ae01f5f..5ba82f76ad 100644 --- a/python/lsst/daf/butler/registry/tests/_registry.py +++ b/python/lsst/daf/butler/registry/tests/_registry.py @@ -2765,6 +2765,18 @@ def testQueryResultSummaries(self): self.assertTrue(messages) self.assertTrue(any("no-purpose" in message for message in messages)) + def testQueryDataIdsExpressionError(self): + """Test error checking of 'where' expressions in queryDataIds.""" + registry = self.makeRegistry() + self.loadData(registry, "base.yaml") + bind = {"time": astropy.time.Time("2020-01-01T01:00:00", format="isot", scale="tai")} + with self.assertRaisesRegex(LookupError, r"No dimension element with name 'foo' in 'foo\.bar'\."): + registry.queryDataIds(["detector"], where="foo.bar = 12") + with self.assertRaisesRegex( + LookupError, "Dimension element name cannot be inferred in this context." + ): + registry.queryDataIds(["detector"], where="timespan.end < time", bind=bind) + def testQueryDataIdsOrderBy(self): """Test order_by and limit on result returned by queryDataIds().""" registry = self.makeRegistry() @@ -2858,7 +2870,7 @@ def do_query(dimensions=("visit", "tract"), datasets=None, collections=None): list(do_query().order_by(order_by)) for order_by in ("undimension.name", "-undimension.name"): - with self.assertRaisesRegex(ValueError, "Unknown dimension element name 'undimension'"): + with self.assertRaisesRegex(ValueError, "Unknown dimension element 'undimension'"): list(do_query().order_by(order_by)) for order_by in ("attract", "-attract"): @@ -2868,7 +2880,11 @@ def do_query(dimensions=("visit", "tract"), datasets=None, collections=None): with self.assertRaisesRegex(ValueError, "Metadata 'exposure_time' exists in more than one dimension"): list(do_query(("exposure", "visit")).order_by("exposure_time")) - with self.assertRaisesRegex(ValueError, "Timespan exists in more than one dimesion"): + with self.assertRaisesRegex( + ValueError, + r"Timespan exists in more than one dimension element \(exposure, visit\); " + r"qualify timespan with specific dimension name\.", + ): list(do_query(("exposure", "visit")).order_by("timespan.begin")) with self.assertRaisesRegex( @@ -2882,6 +2898,11 @@ def do_query(dimensions=("visit", "tract"), datasets=None, collections=None): with self.assertRaisesRegex(ValueError, "Field 'name' does not exist in 'tract'."): list(do_query("tract").order_by("tract.name")) + with self.assertRaisesRegex( + ValueError, r"Unknown dimension element 'timestamp'; perhaps you meant 'timespan.begin'\?" + ): + list(do_query("visit").order_by("timestamp.begin")) + def testQueryDataIdsGovernorExceptions(self): """Test exceptions raised by queryDataIds() for incorrect governors.""" registry = self.makeRegistry() @@ -3001,6 +3022,14 @@ def do_query(element, datasets=None, collections=None): with self.assertRaisesRegex(ValueError, "Field 'attract' does not exist in 'detector'."): list(do_query("detector").order_by(order_by)) + for order_by in ("timestamp.begin", "-timestamp.begin"): + with self.assertRaisesRegex( + ValueError, + r"Element name mismatch: 'timestamp' instead of 'visit'; " + r"perhaps you meant 'timespan.begin'\?", + ): + list(do_query("visit").order_by(order_by)) + def testQueryDimensionRecordsExceptions(self): """Test exceptions raised by queryDimensionRecords().""" registry = self.makeRegistry()