Improve metadata error message to identify the error

pydough/errors/error_utils.py

@@ -32,6 +32,8 @@
 ]
 
 
+import builtins
+import keyword
 from abc import ABC, abstractmethod
 
 import numpy as np
@@ -98,10 +100,15 @@ class ValidName(PyDoughPredicate):
     """
 
     def accept(self, obj: object) -> bool:
-        return isinstance(obj, str) and obj.isidentifier()
+        return (
+            isinstance(obj, str)
+            and obj.isidentifier()
+            and not keyword.iskeyword(obj)
+            and not hasattr(builtins, obj)
+        )
 
     def error_message(self, error_name: str) -> str:
-        return f"{error_name} must be a string that is a Python identifier"
+        return f"{error_name} must be a string that is a valid Python identifier"
 
 
 class NoExtraKeys(PyDoughPredicate):

pydough/metadata/collections/collection_metadata.py

@@ -48,8 +48,8 @@ def __init__(
             PropertyMetadata,
         )
 
-        is_valid_name.verify(name, "name")
-        HasType(GraphMetadata).verify(graph, "graph")
+        is_valid_name.verify(name, f"collection name {name!r}")
+        HasType(GraphMetadata).verify(graph, f"graph {name!r}")
 
         self._graph: GraphMetadata = graph
         self._name: str = name

pydough/metadata/graphs/graph_metadata.py

@@ -43,7 +43,7 @@ def __init__(
         synonyms: list[str] | None,
         extra_semantic_info: dict | None,
     ):
-        is_valid_name.verify(name, "graph name")
+        is_valid_name.verify(name, f"graph name {name!r}")
         self._additional_definitions: list[str] | None = additional_definitions
         self._verified_pydough_analysis: list[dict] | None = verified_pydough_analysis
         self._name: str = name
@@ -179,7 +179,7 @@ def add_function(self, name: str, function: "ExpressionFunctionOperator") -> Non
             `PyDoughMetadataException`: if `function` cannot be inserted
             into the graph because of a name collision.
         """
-        is_valid_name.verify(name, "function name")
+        is_valid_name.verify(name, f"function name {name!r}")
         if name == self.name:
             raise PyDoughMetadataException(
                 f"Function name {name!r} cannot be the same as the graph name {self.name!r}"

pydough/metadata/properties/property_metadata.py

@@ -45,8 +45,8 @@ def __init__(
         synonyms: list[str] | None,
         extra_semantic_info: dict | None,
     ):
-        is_valid_name.verify(name, "name")
-        HasType(CollectionMetadata).verify(collection, "collection")
+        is_valid_name.verify(name, f"property name {name!r}")
+        HasType(CollectionMetadata).verify(collection, f"collection {name}")
         self._name: str = name
         self._collection: CollectionMetadata = collection
         super().__init__(description, synonyms, extra_semantic_info)

tests/test_metadata/invalid_graphs.json

@@ -52,7 +52,7 @@
       "version": "V2",
       "collections": [
         {
-          "name": "0",
+          "name": "Invalid name",
           "type": "simple table",
           "table path": "TableName",
           "unique properties": ["key"],
@@ -66,7 +66,7 @@
       "version": "V2",
       "collections": [
         {
-          "name": "0",
+          "name": "table_name",
           "type": "simple table",
           "table path": "TableName",
           "unique properties": ["key"],
@@ -93,6 +93,94 @@
       ],
       "relationships": []
     },
+    {
+      "name": "BAD_TABLE_NAME",
+      "version": "V2",
+      "collections": [
+        {
+          "name": "invalid table_name",
+          "type": "simple table",
+          "table path": "TableName",
+          "unique properties": ["column_name"],
+          "properties": [
+            {
+              "name": "column_name",
+              "type": "table column",
+              "column name": "column_name",
+              "data type": "string",
+              "description": "column description"
+            }
+          ]
+        }
+      ],
+      "relationships": []
+    },
+    {
+      "name": "BAD_COLUMN_NAME",
+      "version": "V2",
+      "collections": [
+        {
+          "name": "collection",
+          "type": "simple table",
+          "table path": "TableName",
+          "unique properties": ["key"],
+          "properties": [
+            {
+              "name": "invalid column name",
+              "type": "table column",
+              "column name": "column_name",
+              "data type": "string",
+              "description": "column description"
+            }
+          ]
+        }
+      ],
+      "relationships": []
+    },
+    {
+      "name": "BAD_UNIQUE_PROPERTY",
+      "version": "V2",
+      "collections": [
+        {
+          "name": "collection",
+          "type": "simple table",
+          "table path": "TableName",
+          "unique properties": ["invalid column name"],
+          "properties": [
+            {
+              "name": "invalid column name",
+              "type": "table column",
+              "column name": "column_name",
+              "data type": "string",
+              "description": "column description"
+            }
+          ]
+        }
+      ],
+      "relationships": []
+    },
+    {
+      "name": "RESERVED_PROPERTY_KEYWORD",
+      "version": "V2",
+      "collections": [
+        {
+          "name": "collection",
+          "type": "simple table",
+          "table path": "TableName",
+          "unique properties": ["id"],
+          "properties": [
+            {
+              "name": "id",
+              "type": "table column",
+              "column name": "id",
+              "data type": "numeric",
+              "description": "column description"
+            }
+          ]
+        }
+      ],
+      "relationships": []
+    },
     {
       "name": "BAD_RELATIONSHIP_NAME",
       "version": "V2",

tests/test_metadata_errors.py

@@ -53,7 +53,7 @@ def test_missing_property(get_sample_graph: graph_fetcher) -> None:
         ),
         pytest.param(
             "#BadGraphName",
-            "graph name must be a string that is a Python identifier",
+            "graph name '#BadGraphName' must be a string that is a valid Python identifier",
             id="#BadGraphName",
         ),
         pytest.param(
@@ -85,24 +85,44 @@ def test_missing_property(get_sample_graph: graph_fetcher) -> None:
         ),
         pytest.param(
             "BAD_COLLECTION_NAME_1",
-            "name must be a string that is a Python identifier",
+            "collection name '0' must be a string that is a valid Python identifier",
             id="BAD_COLLECTION_NAME_1",
         ),
         pytest.param(
             "BAD_COLLECTION_NAME_2",
-            "name must be a string that is a Python identifier",
+            "collection name 'Invalid name' must be a string that is a valid Python identifier",
             id="BAD_COLLECTION_NAME_2",
         ),
         pytest.param(
             "BAD_PROPERTY_NAME_1",
-            "name must be a string that is a Python identifier",
+            "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",
             id="BAD_PROPERTY_NAME_1",
         ),
         pytest.param(
             "BAD_PROPERTY_NAME_2",
-            "name must be a string that is a Python identifier",
+            "collection name '0' must be a string that is a valid Python identifier",
             id="BAD_PROPERTY_NAME_2",
         ),
+        pytest.param(
+            "BAD_TABLE_NAME",
+            "collection name 'invalid table_name' must be a string that is a valid Python identifier",
+            id="BAD_TABLE_NAME",
+        ),
+        pytest.param(
+            "BAD_COLUMN_NAME",
+            "property name 'invalid column name' must be a string that is a valid Python identifier",
+            id="BAD_COLUMN_NAME",
+        ),
+        pytest.param(
+            "BAD_UNIQUE_PROPERTY",
+            "property name 'invalid column name' must be a string that is a valid Python identifier",
+            id="BAD_UNIQUE_PROPERTY",
+        ),
+        pytest.param(
+            "RESERVED_PROPERTY_KEYWORD",
+            "property name 'id' must be a string that is a valid Python identifier",
+            id="RESERVED_PROPERTY_KEYWORD",
+        ),
         pytest.param(
             "BAD_RELATIONSHIP_NAME",
             "metadata for relationships within graph 'BAD_RELATIONSHIP_NAME' must be a JSON object containing a field 'type' and field 'type' must be a string",
@@ -463,7 +483,7 @@ def test_missing_property(get_sample_graph: graph_fetcher) -> None:
         ),
         pytest.param(
             "BAD_FUNCTION_BAD_NAME_2",
-            "function name must be a string that is a Python identifier",
+            "function name '' must be a string that is a valid Python identifier",
             id="BAD_FUNCTION_BAD_NAME_2",
         ),
         pytest.param(