Skip to content

Commit 161155b

Browse files
committed
Improve metadata error messages
1 parent 5480941 commit 161155b

File tree

6 files changed

+63
-17
lines changed

6 files changed

+63
-17
lines changed

pydough/errors/error_utils.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ def verify(self, obj: object, error_name: str) -> None:
8989
`PyDoughMetadataException`: if `obj` did not satisfy the predicate.
9090
"""
9191
if not self.accept(obj):
92-
error_name = f"{error_name}: [{repr(obj)}]"
9392
raise PyDoughMetadataException(self.error_message(error_name))
9493

9594

pydough/metadata/collections/collection_metadata.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ def __init__(
4848
PropertyMetadata,
4949
)
5050

51-
is_valid_name.verify(name, "name")
52-
HasType(GraphMetadata).verify(graph, "graph")
51+
is_valid_name.verify(name, f"name {name!r}")
52+
HasType(GraphMetadata).verify(graph, f"graph {graph}")
5353

5454
self._graph: GraphMetadata = graph
5555
self._name: str = name

pydough/metadata/graphs/graph_metadata.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ def __init__(
4343
synonyms: list[str] | None,
4444
extra_semantic_info: dict | None,
4545
):
46-
breakpoint()
47-
is_valid_name.verify(name, "graph name")
46+
is_valid_name.verify(name, f"graph name {name!r}")
4847
self._additional_definitions: list[str] | None = additional_definitions
4948
self._verified_pydough_analysis: list[dict] | None = verified_pydough_analysis
5049
self._name: str = name
@@ -117,7 +116,7 @@ def add_collection(self, collection: AbstractMetadata) -> None:
117116
from pydough.metadata.collections import CollectionMetadata
118117

119118
# Make sure the collection is actually a collection
120-
HasType(CollectionMetadata).verify(collection, "collection")
119+
HasType(CollectionMetadata).verify(collection, f"collection {collection!r}")
121120
assert isinstance(collection, CollectionMetadata)
122121

123122
# Verify sure the collection has not already been added to the graph
@@ -180,7 +179,7 @@ def add_function(self, name: str, function: "ExpressionFunctionOperator") -> Non
180179
`PyDoughMetadataException`: if `function` cannot be inserted
181180
into the graph because of a name collision.
182181
"""
183-
is_valid_name.verify(name, "function name")
182+
is_valid_name.verify(name, f"function name {name!r}")
184183
if name == self.name:
185184
raise PyDoughMetadataException(
186185
f"Function name {name!r} cannot be the same as the graph name {self.name!r}"

pydough/metadata/properties/property_metadata.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ def __init__(
4545
synonyms: list[str] | None,
4646
extra_semantic_info: dict | None,
4747
):
48-
is_valid_name.verify(name, "name")
49-
HasType(CollectionMetadata).verify(collection, "collection")
48+
is_valid_name.verify(name, f"name {name!r}")
49+
HasType(CollectionMetadata).verify(collection, f"collection {collection!r}")
5050
self._name: str = name
5151
self._collection: CollectionMetadata = collection
5252
super().__init__(description, synonyms, extra_semantic_info)

tests/test_metadata/invalid_graphs.json

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"version": "V2",
5353
"collections": [
5454
{
55-
"name": "0",
55+
"name": "Invalid name",
5656
"type": "simple table",
5757
"table path": "TableName",
5858
"unique properties": ["key"],
@@ -66,7 +66,7 @@
6666
"version": "V2",
6767
"collections": [
6868
{
69-
"name": "0",
69+
"name": "table_name",
7070
"type": "simple table",
7171
"table path": "TableName",
7272
"unique properties": ["key"],
@@ -93,6 +93,44 @@
9393
],
9494
"relationships": []
9595
},
96+
{
97+
"name": "BAD_COLUMN_NAME",
98+
"version": "V2",
99+
"collections": [
100+
{
101+
"name": "collection",
102+
"type": "simple table",
103+
"table path": "TableName",
104+
"unique properties": ["key"],
105+
"properties": [
106+
{
107+
"name": "invalid column name",
108+
"type": "table column",
109+
"column name": "column_name",
110+
"data type": "string",
111+
"description": "column description"
112+
}
113+
]
114+
}
115+
],
116+
"relationships": []
117+
},
118+
{
119+
"name": "BAD_TABLE_NAME",
120+
"version": "V2",
121+
"collections": [
122+
{
123+
"name": "invalid table_name",
124+
"type": "simple table",
125+
"table path": "TableName",
126+
"unique properties": ["key"],
127+
"properties": [
128+
{"name": "property"}
129+
]
130+
}
131+
],
132+
"relationships": []
133+
},
96134
{
97135
"name": "BAD_RELATIONSHIP_NAME",
98136
"version": "V2",

tests/test_metadata_errors.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def test_missing_property(get_sample_graph: graph_fetcher) -> None:
5353
),
5454
pytest.param(
5555
"#BadGraphName",
56-
"graph name must be a string that is a Python identifier",
56+
"graph name '#BadGraphName' must be a string that is a valid Python identifier",
5757
id="#BadGraphName",
5858
),
5959
pytest.param(
@@ -85,22 +85,32 @@ def test_missing_property(get_sample_graph: graph_fetcher) -> None:
8585
),
8686
pytest.param(
8787
"BAD_COLLECTION_NAME_1",
88-
"name must be a string that is a Python identifier",
88+
"name '0' must be a string that is a valid Python identifier",
8989
id="BAD_COLLECTION_NAME_1",
9090
),
9191
pytest.param(
9292
"BAD_COLLECTION_NAME_2",
93-
"name must be a string that is a Python identifier",
93+
"name 'Invalid name' must be a string that is a valid Python identifier",
9494
id="BAD_COLLECTION_NAME_2",
9595
),
9696
pytest.param(
9797
"BAD_PROPERTY_NAME_1",
98-
"name must be a string that is a Python identifier",
98+
"property 'bad name' of simple table collection 'table_name' in graph 'BAD_PROPERTY_NAME_1' must be a JSON object containing a field 'type' and field 'type' must be a string",
9999
id="BAD_PROPERTY_NAME_1",
100100
),
101101
pytest.param(
102102
"BAD_PROPERTY_NAME_2",
103-
"name must be a string that is a Python identifier",
103+
"name '0' must be a string that is a valid Python identifier",
104+
id="BAD_PROPERTY_NAME_2",
105+
),
106+
pytest.param(
107+
"BAD_COLUMN_NAME",
108+
"name 'invalid column name' must be a string that is a valid Python identifier",
109+
id="BAD_PROPERTY_NAME_2",
110+
),
111+
pytest.param(
112+
"BAD_TABLE_NAME",
113+
"name 'invalid table_name' must be a string that is a valid Python identifier",
104114
id="BAD_PROPERTY_NAME_2",
105115
),
106116
pytest.param(
@@ -463,7 +473,7 @@ def test_missing_property(get_sample_graph: graph_fetcher) -> None:
463473
),
464474
pytest.param(
465475
"BAD_FUNCTION_BAD_NAME_2",
466-
"function name must be a string that is a Python identifier",
476+
"function name '' must be a string that is a valid Python identifier",
467477
id="BAD_FUNCTION_BAD_NAME_2",
468478
),
469479
pytest.param(

0 commit comments

Comments
 (0)