Skip to content

Commit

Permalink
Attempt to provide helpful guesses about intent in error messages.
Browse files Browse the repository at this point in the history
  • Loading branch information
TallJimbo committed Jul 13, 2023
1 parent 0615ff0 commit db557ce
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
33 changes: 24 additions & 9 deletions python/lsst/daf/butler/registry/queries/expressions/categorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Check warning on line 95 in python/lsst/daf/butler/registry/queries/expressions/categorize.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/queries/expressions/categorize.py#L95

Added line #L95 was not covered by tests
if table == "timespan" or table == "datetime" or table == "timestamp":
raise LookupError(

Check warning on line 97 in python/lsst/daf/butler/registry/queries/expressions/categorize.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/queries/expressions/categorize.py#L97

Added line #L97 was not covered by tests
"Dimension element name cannot be inferred in this context; "
f"use <dimension>.timespan.{column} instead."
) from None
raise LookupError(f"No dimension element with name {table!r} in {name!r}.") from None

Check warning on line 101 in python/lsst/daf/butler/registry/queries/expressions/categorize.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/queries/expressions/categorize.py#L101

Added line #L101 was not covered by tests
if isinstance(element, Dimension) and column == element.primaryKey.name:
# Allow e.g. "visit.id = x" instead of just "visit = x"; this
# can be clearer.
Expand Down Expand Up @@ -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}'.")
Expand All @@ -197,16 +203,21 @@ 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.")
else:
# 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(

Check warning on line 217 in python/lsst/daf/butler/registry/queries/expressions/categorize.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/queries/expressions/categorize.py#L217

Added line #L217 was not covered by tests
f"Unknown dimension element name {elem_name!r}; perhaps you meant 'timespan'?"
)
raise ValueError(f"Unknown dimension element name {elem_name!r}.")
element = graph.elements[elem_name]
if field_name in ("timespan.begin", "timespan.end"):
if not element.temporal:
Expand Down Expand Up @@ -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}'?"

Check warning on line 304 in python/lsst/daf/butler/registry/queries/expressions/categorize.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/queries/expressions/categorize.py#L304

Added line #L304 was not covered by tests
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}'.")
Expand Down
6 changes: 5 additions & 1 deletion python/lsst/daf/butler/registry/tests/_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -2867,7 +2867,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(
Expand Down

0 comments on commit db557ce

Please sign in to comment.