Repository: SINTEF/dlite (language: python)

Issue:
Datamodel dimensions 
When adding dimensions to the datamodel that are not used in the properties, no error is returned. 
This should not be valid. I suggest checking that all dimensions added are actually made use for, if not the datamodel should not contain it.


Discussion / hints:

Fix patch:
diff --git a/bindings/python/CMakeLists.txt b/bindings/python/CMakeLists.txt
index de7bbadee..c1c89a583 100644
--- a/bindings/python/CMakeLists.txt
+++ b/bindings/python/CMakeLists.txt
@@ -215,14 +215,16 @@ install(
   DESTINATION ${Python3_MODULE_PATH}
   PATTERN ".gitignore" EXCLUDE
   PATTERN "*~" EXCLUDE
-  )
-
+)
 install(
   DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/python-mapping-plugins
   DESTINATION share/dlite
   PATTERN ".gitignore" EXCLUDE
   PATTERN "*~" EXCLUDE
-  )
+)
+
+
+add_subdirectory(scripts)
 
 
 #
diff --git a/bindings/python/scripts/CMakeLists.txt b/bindings/python/scripts/CMakeLists.txt
new file mode 100644
index 000000000..e09509f54
--- /dev/null
+++ b/bindings/python/scripts/CMakeLists.txt
@@ -0,0 +1,67 @@
+project(dlite-bindings-python-scripts C)
+
+# Convinience functions for check for success
+# Usage: test_success(name, command...)
+function(test_success name)
+add_test(
+  NAME ${name}
+  COMMAND ${RUNNER} ${Python3_EXECUTABLE} ${ARGN}
+  WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+)
+endfunction()
+
+# Convinience functions for check for failure
+# Usage: test_failure(name, command...)
+function(test_failure name)
+  test_success(${name} ${ARGN})
+  set_property(TEST ${name} PROPERTY WILL_FAIL true)
+endfunction()
+
+
+# Install
+install(
+  CODE "
+   execute_process(
+     COMMAND ${Python3_EXECUTABLE} setup.py install
+       --prefix=${CMAKE_INSTALL_PREFIX} --force
+     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+   )"
+)
+
+
+# Tests
+test_success(
+  dlite-validate-Person
+  dlite-validate ../tests/Person.json
+)
+test_success(
+  dlite-validate-persons
+  dlite-validate -p ../tests/Person.json -i 51c0d700-9ab0-43ea-9183-6ea22012ebee
+    ../tests/persons.json
+)
+test_success(
+  dlite-validate-persons2
+  dlite-validate
+    --storage-path=../tests/Person.json
+    --id=13c5007b-c665-4709-a11c-255919306547
+    --show
+    ../tests/persons.json
+  )
+
+# No --id for storage with multiple instances
+test_failure(
+  dlite-validate-persons-fail2
+  dlite-validate --storage-path ../tests/Person.json --show ../tests/persons.json
+)
+
+# The "dimensions" keyword is required
+test_failure(
+  dlite-validate-no-dimensions
+  dlite-validate NoDimensions.json
+)
+
+# Ok with an empty entity as long as the "dimensions" and "properties" keys are there.
+test_success(
+  dlite-validate-empty
+  dlite-validate Empty.json
+)
diff --git a/bindings/python/scripts/Empty.json b/bindings/python/scripts/Empty.json
new file mode 100644
index 000000000..e08fb0abe
--- /dev/null
+++ b/bindings/python/scripts/Empty.json
@@ -0,0 +1,6 @@
+{
+  "uri": "http://onto-ns.com/meta/0.1/Empty",
+  "description": "Empty entity.",
+  "dimensions": {},
+  "properties": {}
+}
diff --git a/bindings/python/scripts/NoDimensions.json b/bindings/python/scripts/NoDimensions.json
new file mode 100644
index 000000000..05be042cb
--- /dev/null
+++ b/bindings/python/scripts/NoDimensions.json
@@ -0,0 +1,12 @@
+{
+  "uri": "http://onto-ns.com/meta/0.1/NoDimensions",
+  "description": "Entity without dimensions.",
+  "properties": {
+    "prop1": {
+      "type": "string"
+    },
+    "prop2": {
+      "type": "int"
+    }
+  }
+}
diff --git a/bindings/python/scripts/dlite-validate b/bindings/python/scripts/dlite-validate
new file mode 100644
index 000000000..13c10e208
--- /dev/null
+++ b/bindings/python/scripts/dlite-validate
@@ -0,0 +1,123 @@
+#!/usr/bin/env python3
+"""Validator for dlite datamodels.
+"""
+import sys
+import argparse
+import json
+import re
+import yaml
+from pathlib import Path
+
+import dlite
+
+
+def parse(url, driver=None, options=None, id=None):
+    """Loads an instance from storage.
+
+    Arguments:
+        url: URL of storage to load from.
+        driver: The name of storage plugin to use.
+        options: Options passed to the storage plugin.
+        id: ID of instance to load if the storage contains multiple instances.
+
+    Returns:
+        A new instance.
+    """
+    match = re.match(r'^([a-zA-Z][a-zA-Z0-9+.-]*)://', url)
+    if match and match.groups()[0].lower() != 'file':
+        if driver:
+            return dlite.Instance.from_location(
+                driver, url, options=options, id=id,
+            )
+        else:
+            return dlite.Instance.from_url(f'{url}#{id}' if id else url)
+    else:
+        path = url[7:] if url.lower().startswith('file://') else url
+        if driver:
+            return dlite.Instance_from_location(
+                driver, path, options=options, id=id
+            )
+        fileext = Path(path).suffix.lstrip('.').lower()
+
+        # For json and yaml, use the Python parser instead of dlite, since
+        # they have better error reporting for invalid serialisation
+        if fileext == 'json':
+            with open(path, 'rt') as f:
+                return dlite.Instance.from_dict(json.load(f), id=id)
+        elif fileext in ('yaml', 'yml'):
+            with open(path, 'rt') as f:
+                return dlite.Instance.from_dict(yaml.safe_load(f), id=id)
+        return dlite.Instance.from_location(fileext, path, options=options)
+
+
+def check_dimensions(url, meta):
+    """Check wheter all dimensions in metadata are referred to in shapes."""
+    shape_dims = set()
+    for prop in meta.properties['properties']:
+        shape_dims.update(prop.shape)
+    unused = set(meta.dimnames()).difference(shape_dims)
+    if unused:
+        print(f'{url}: unused dimensions: {", ".join(unused)}')
+        sys.exit(1)
+
+
+def main():
+    """Datamodel validator"""
+    parser = argparse.ArgumentParser(
+        description='DLite datamodel validator.',
+    )
+    parser.add_argument(
+        'url',
+        help='URL to datamodel to validate.  Relative file paths are allowed.'
+    )
+    parser.add_argument(
+        '--driver', '--plugin', '-d',
+        help='Storage plugin to use when loading the instance.  If `url` '
+        'is a file path, the default is to infer the driver from the file '
+        'extension.',
+    )
+    parser.add_argument(
+        '--options', '-o',
+        help='Semicolon-separated list of options parsed on to the dlite '
+        'storage plugin.  For instance, to validate an entity in a '
+        '"mongodb" storage, you may want to provide '
+        '`--options="database=DBNAME;collection=dlite_coll;user=myname"`.'
+    )
+    parser.add_argument(
+        '--id', '-i',
+        help='Id of instance to load if the storage contains multiple '
+        'instances.'
+    )
+    parser.add_argument(
+        '--storage-path', '-p', action='append', default=[],
+        help='URL or path to storage path to look up metadata. '
+        'Only needed when validating data instances. '
+        'This option can be given multiple times.'
+    )
+    parser.add_argument(
+        '--show', '-s', action='store_true',
+        help='Print the json-serialisation of the instance to standard output.'
+    )
+
+    args = parser.parse_args()
+
+    for path in args.storage_path:
+        dlite.storage_path.append(Path(path).resolve())
+
+    try:
+        inst = parse(args.url, args.driver, args.options, args.id)
+    except Exception as exc:
+        print(f'{args.url}: {exc.__class__.__name__}: {exc}')
+        sys.exit(1)
+
+    if inst.is_meta:
+        check_dimensions(args.url, inst)
+
+    print(f'{args.url}: validated')
+
+    if (args.show):
+        print(inst)
+
+
+if __name__ == '__main__':
+    main()
diff --git a/bindings/python/scripts/setup.py b/bindings/python/scripts/setup.py
new file mode 100644
index 000000000..3250b8fb2
--- /dev/null
+++ b/bindings/python/scripts/setup.py
@@ -0,0 +1,23 @@
+"""setup.py file for installing scripts."""
+import re
+import sys
+from setuptools import setup
+from glob import glob
+
+
+with open('../../../CMakeLists.txt', 'rt') as f:
+    match = re.search(r'VERSION +([0-9.]+)', f.read())
+    version, = match.groups()
+
+
+setup(
+    name='dlite',
+    version=version,
+    scripts=['dlite-validate'],
+    entry_points={
+        'console_scripts': [
+            'pip=pip:main',
+            f'pip-{sys.version[:3]}=pip:main',
+        ],
+    },
+)


Test patch:
diff --git a/bindings/python/tests/persons.json b/bindings/python/tests/persons.json
index a875e83ef..044e50025 100644
--- a/bindings/python/tests/persons.json
+++ b/bindings/python/tests/persons.json
@@ -1,5 +1,5 @@
 {
-  "51c0d700-9ab0-43ea-9183-6ea22012ebee":   {
+  "51c0d700-9ab0-43ea-9183-6ea22012ebee": {
     "meta": "http://onto-ns.com/meta/0.1/Person",
     "dimensions": {
       "N": 2
@@ -9,5 +9,16 @@
       "age": 42,
       "skills": ["distilling", "tasting"]
     }
+  },
+  "13c5007b-c665-4709-a11c-255919306547": {
+    "meta": "http://onto-ns.com/meta/0.1/Person",
+    "dimensions": {
+      "N": 3
+    },
+    "properties": {
+      "name": "Jon Doe",
+      "age": 32,
+      "skills": ["reading", "talking", "sleeping"]
+    }
   }
 }