Merge pull request #56 from AustralianBioCommons/fix-array-should-have-items

Fix array should have items

Author: JoshuaHarris391

docs/gen3schemadev/first_dictionary.md

@@ -176,9 +176,7 @@ Properties are found within nodes. Properties can be thought of as the columns i
 | integer    | Whole numbers                                    | 42                   |
 | number     | Numeric values (integer or floating point)        | 3.14                 |
 | boolean    | True or false values                             | true                 |
-| object     | Key-value pairs (dictionary/map)                  | {"age": 30}          |
 | array      | Ordered list of values                           | [1, 2, 3]            |
-| null       | Null value (no value)                            | null                 |
 | datetime   | ISO 8601 date/time string (special type recognised by Gen3SchemaDev) | "2024-01-01T12:00:00Z" |
 
 *These are the standard [JSON Schema data types](https://json-schema.org/understanding-json-schema/reference/type.html) used to define the kind of data a property can hold.*

jupyter/protoyping.ipynb

@@ -12,27 +12,25 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 2,
+   "execution_count": 3,
    "id": "7bda6e32",
    "metadata": {},
    "outputs": [
     {
-     "ename": "ValueError",
-     "evalue": "Schema 'lipidomics_file' with category 'data_file' must include properties 'data_type', 'data_format', and 'data_category'. Please add these properties to the 'properties' section.",
-     "output_type": "error",
-     "traceback": [
-      "\u001b[31m---------------------------------------------------------------------------\u001b[39m",
-      "\u001b[31mValueError\u001b[39m                                Traceback (most recent call last)",
-      "\u001b[36mCell\u001b[39m\u001b[36m \u001b[39m\u001b[32mIn[2]\u001b[39m\u001b[32m, line 3\u001b[39m\n\u001b[32m      1\u001b[39m lipid_schema = g.utils.load_yaml(\u001b[33m\"\u001b[39m\u001b[33m../output/lipidomics_file.yaml\u001b[39m\u001b[33m\"\u001b[39m)\n\u001b[32m      2\u001b[39m rule_val = g.validators.rule_validator.RuleValidator(lipid_schema)\n\u001b[32m----> \u001b[39m\u001b[32m3\u001b[39m \u001b[43mrule_val\u001b[49m\u001b[43m.\u001b[49m\u001b[43mdata_file_props_need_data_props\u001b[49m\u001b[43m(\u001b[49m\u001b[43m)\u001b[49m\n",
-      "\u001b[36mFile \u001b[39m\u001b[32m~/projects/gen3schemadev/src/gen3schemadev/validators/rule_validator.py:208\u001b[39m, in \u001b[36mRuleValidator.data_file_props_need_data_props\u001b[39m\u001b[34m(self)\u001b[39m\n\u001b[32m    206\u001b[39m \u001b[38;5;28;01mif\u001b[39;00m \u001b[38;5;129;01mnot\u001b[39;00m need_props.issubset(prop_keys):\n\u001b[32m    207\u001b[39m     schema_id = \u001b[38;5;28mself\u001b[39m.schema.get(\u001b[33m\"\u001b[39m\u001b[33mid\u001b[39m\u001b[33m\"\u001b[39m, \u001b[33m\"\u001b[39m\u001b[33m<unknown id>\u001b[39m\u001b[33m\"\u001b[39m)\n\u001b[32m--> \u001b[39m\u001b[32m208\u001b[39m     \u001b[38;5;28;01mraise\u001b[39;00m \u001b[38;5;167;01mValueError\u001b[39;00m(\n\u001b[32m    209\u001b[39m         \u001b[33mf\u001b[39m\u001b[33m\"\u001b[39m\u001b[33mSchema \u001b[39m\u001b[33m'\u001b[39m\u001b[38;5;132;01m{\u001b[39;00mschema_id\u001b[38;5;132;01m}\u001b[39;00m\u001b[33m'\u001b[39m\u001b[33m with category \u001b[39m\u001b[33m'\u001b[39m\u001b[33mdata_file\u001b[39m\u001b[33m'\u001b[39m\u001b[33m must include properties \u001b[39m\u001b[33m\"\u001b[39m\n\u001b[32m    210\u001b[39m         \u001b[33mf\u001b[39m\u001b[33m\"\u001b[39m\u001b[33m'\u001b[39m\u001b[33mdata_type\u001b[39m\u001b[33m'\u001b[39m\u001b[33m, \u001b[39m\u001b[33m'\u001b[39m\u001b[33mdata_format\u001b[39m\u001b[33m'\u001b[39m\u001b[33m, and \u001b[39m\u001b[33m'\u001b[39m\u001b[33mdata_category\u001b[39m\u001b[33m'\u001b[39m\u001b[33m. Please add these properties \u001b[39m\u001b[33m\"\u001b[39m\n\u001b[32m    211\u001b[39m         \u001b[33mf\u001b[39m\u001b[33m\"\u001b[39m\u001b[33mto the \u001b[39m\u001b[33m'\u001b[39m\u001b[33mproperties\u001b[39m\u001b[33m'\u001b[39m\u001b[33m section.\u001b[39m\u001b[33m\"\u001b[39m\n\u001b[32m    212\u001b[39m     )\n\u001b[32m    213\u001b[39m \u001b[38;5;28;01mreturn\u001b[39;00m \u001b[38;5;28;01mTrue\u001b[39;00m\n",
-      "\u001b[31mValueError\u001b[39m: Schema 'lipidomics_file' with category 'data_file' must include properties 'data_type', 'data_format', and 'data_category'. Please add these properties to the 'properties' section."
-     ]
+     "data": {
+      "text/plain": [
+       "True"
+      ]
+     },
+     "execution_count": 3,
+     "metadata": {},
+     "output_type": "execute_result"
     }
    ],
    "source": [
     "lipid_schema = g.utils.load_yaml(\"../output/lipidomics_file.yaml\")\n",
     "rule_val = g.validators.rule_validator.RuleValidator(lipid_schema)\n",
-    "rule_val.data_file_props_need_data_props()"
+    "rule_val.type_array_needs_items()"
    ]
   },
   {

src/gen3schemadev/converter.py

@@ -524,6 +524,65 @@ def format_datetime(prop_dict: dict) -> dict:
         raise RuntimeError(f"Error formatting datetime property: {e}") from e
 
 
+def format_array(prop_dict: dict) -> dict:
+    """
+    Formats a property dictionary with a 'type' of 'array' to use a Gen3 'array' schema definition.
+
+    If the property is not of type 'array', returns the property unchanged.
+
+    Example input:
+        {
+            "sample_ids": {
+                "type": "array",
+                "description": "Sample IDs (array)"
+            }
+        }
+
+    Example output:
+        {
+            "sample_ids": {
+                "type": "array",
+                "items": {
+                    "type": "string"
+                }
+            }
+        }
+
+    Args:
+        prop_dict (dict): A dictionary with a single property as key and its attributes as value.
+
+    Returns:
+        dict: The formatted property dictionary suitable for Gen3 schema usage.
+
+    Raises:
+        ValueError: If the input dictionary does not contain exactly one property.
+        RuntimeError: If an error occurs during formatting.
+    """
+    try:
+        if len(prop_dict) != 1:
+            raise ValueError("Expected a single property dictionary")
+
+        key, value = next(iter(prop_dict.items()))
+
+        if isinstance(value, dict) and value.get('type') == 'array':
+            # Safely preserve description and required if present
+            formatted = {
+                "type": "array",
+                "items": {
+                    "type": "string"
+                }
+            }
+            if "description" in value:
+                formatted["description"] = value["description"]
+            if "required" in value:
+                formatted["required"] = value["required"]
+            return {key: formatted}
+        else:
+            return {key: value}
+    except Exception as e:
+        raise RuntimeError(f"Error formatting array property: {e}") from e
+
+
 def construct_props(node_name: str, data: DataSourceProtocol) -> dict:
     """
     Construct the 'properties' section for a Gen3 schema node.
@@ -554,6 +613,7 @@ def construct_props(node_name: str, data: DataSourceProtocol) -> dict:
         if isinstance(prop, dict):
             prop = format_enum(prop)
             prop = format_datetime(prop)
+            prop = format_array(prop)
             props_dict.update(prop)
 
     # Add link properties

src/gen3schemadev/schema/input_schema.py

@@ -12,7 +12,7 @@ class Property(BaseModel):
     """Schema for gen3 property"""
     name: str = Field(description="The name of the property.")
     type: Literal[
-        'string', 'integer', 'number', 'boolean', 'datetime', 'enum'
+        'string', 'integer', 'number', 'boolean', 'datetime', 'enum', 'array'
     ] = Field(description="The data type of the property.")
     description: str = Field(description="A human-readable description of the property.")
     required: bool = Field(default=False, description="Whether this property is required for the node.")

src/gen3schemadev/validators/rule_validator.py

@@ -14,6 +14,7 @@ def validate(self):
         self.link_props_exist()
         self.props_cannot_be_system_props()
         self.props_must_have_type()
+        self.type_array_needs_items()
 
     def _get_links(self):
         links = self.schema.get("links", [])
@@ -211,3 +212,17 @@ def data_file_props_need_data_props(self):
                 f"to the 'properties' section."
             )
         return True
+
+    
+    def type_array_needs_items(self):
+        """Ensure that any property with 'type': 'array' includes an 'items' property."""
+        props = self._get_props()
+
+        for key, value in props.items():
+            if isinstance(value, dict) and value.get("type") == "array":
+                if "items" not in value:
+                    raise ValueError(
+                        f"Schema '{self.schema.get('id')}' property '{key}' with type 'array' must include an 'items' property. "
+                        "Please add an 'items' property to the 'properties' section."
+                    )
+        return True

tests/input_example.yml

@@ -58,6 +58,9 @@ nodes:
       type: number
       description: Coefficient of variation (%)
       required: true
+    - name: lipid_species
+      type: array
+      description: List of lipid species
 
 
 links:

tests/test_converter.py

@@ -464,8 +464,14 @@ def test_construct_prop_lipidomics_file(fixture_input_yaml_pass):
         "data_type": {
             "description": "The type of data in this data file",
             "enum": ['data_type_1', 'data_type_2', 'data_type_3']
+        },
+        "lipid_species": {
+            "description": "List of lipid species",
+            "items": {
+                "type": "string"
+            },
+            "type": "array"
         }
-        
     }
     assert result == expected
 
@@ -614,6 +620,13 @@ def fixture_expected_output_lipid():
             "data_type": {
                 "description": "The type of data in this data file",
                 "enum": ['data_type_1', 'data_type_2', 'data_type_3']
+            },
+            "lipid_species": {
+                "description": "List of lipid species",
+                "items": {
+                    "type": "string"
+                },
+                "type": "array"
             }
         },
         'additionalProperties': False

tests/test_rule_validator.py

@@ -166,3 +166,66 @@ def test_data_file_props_need_data_props():
     assert "data_format" in str(excinfo2.value)
     assert "data_category" in str(excinfo2.value)
 
+
+def test_type_array_needs_items_passes_on_arrays_with_items():
+    schema = {
+        "id": "array_test_schema",
+        "properties": {
+            "string_list": {
+                "type": "array",
+                "items": {"type": "string"}
+            },
+            "int_list": {
+                "type": "array",
+                "items": {"type": "integer"}
+            },
+            "foo": {"type": "string"}
+        }
+    }
+    rv = RuleValidator(schema)
+    assert rv.type_array_needs_items() is True
+
+def test_type_array_needs_items_fails_when_missing_items():
+    schema = {
+        "id": "array_bad_schema",
+        "properties": {
+            "string_list": {
+                "type": "array"
+                # missing "items"
+            },
+            "foo": {"type": "string"}
+        }
+    }
+    rv = RuleValidator(schema)
+    with pytest.raises(ValueError) as excinfo:
+        rv.type_array_needs_items()
+    assert "must include an 'items' property" in str(excinfo.value)
+    assert "string_list" in str(excinfo.value)
+    assert "array_bad_schema" in str(excinfo.value)
+
+def test_type_array_needs_items_ignores_non_array_properties():
+    schema = {
+        "id": "array_ok_schema",
+        "properties": {
+            "foo": {"type": "string"},
+            "bar": {"type": "integer"},
+            "flag": {"type": "boolean"}
+        }
+    }
+    rv = RuleValidator(schema)
+    assert rv.type_array_needs_items() is True
+
+def test_type_array_needs_items_skips_refs():
+    schema = {
+        "id": "array_ref_schema",
+        "properties": {
+            "referenced_prop": {"$ref": "#/definitions/foo"},
+            "another_array": {
+                "type": "array",
+                "items": {"type": "number"}
+            }
+        }
+    }
+    rv = RuleValidator(schema)
+    # The $ref property should be ignored, should not raise
+    assert rv.type_array_needs_items() is True