Repository: ResearchObject/ro-crate-py (python)

Issue:
Bug: Won't load a crate with hasPart reference to a RepositoryCollection.
Loading the below test data gives an error:

```
File "/Users/pt/working/ro-crate-py/test.py", line 3, in <module>
    crate = ROCrate("haspart") 
  File "/Users/pt/rocrate/lib/python3.9/site-packages/rocrate/rocrate.py", line 92, in __init__
    self.build_crate(entities, source_path, gen_preview)
  File "/Users/pt/rocrate/lib/python3.9/site-packages/rocrate/rocrate.py", line 227, in build_crate
    self.add(instance)
UnboundLocalError: local variable 'instance' referenced before assignment

```

I would expect to be able to load anything with a flat JSON-LD structure even if it's not schema.org compliant.


Here's the offending ro-crate-metadata.json

```json
{
  "@context": [
    "https://w3id.org/ro/crate/1.1/context",
    {
      "@vocab": "http://schema.org/"
    },
    {
      "@base": null
    }
  ],
  "@graph": [
    {
      "@id": "#collection",
      "@type": "RepositoryCollection ",
      "name": "Test collection"
    },
    {
      "@id": "./",
      "@type": "Dataset",
      "hasFile": [],
      "hasPart": [{"@id": "#collection"}],
      "name": "testing hasPart"
    },
    {
      "@id": "ro-crate-metadata.json",
      "@type": "CreativeWork",
      "about": {
        "@id": "./"
      },
      "identifier": "ro-crate-metadata.json"
    }
  ]
}
```

Pull request:
Handle unknown data entities
Fixes #74.

The library does not crash anymore if it encounters an "unexpected" data entity, i.e., something linked to from `hasPart` that's not a `File` or `Dataset`. Rather, the entity is added to the crate as a `DataEntity`. No-op `write` and `write_zip` methods have been added to `DataEntity`, to allow such a crate to be written without errors. The ro-crate author can always add a `File` or `Dataset` type to the entity's type list if/when appropriate, in which case the more specific type will be used to instantiate the object and the library will know what to do with it when writing the crate.

For instance, in the following example, a `DataEntity` will be created:

```json
        {
            "@id": "./",
            "@type": "Dataset",
            "hasPart": [{"@id": "#collection"}],
        },
        {
            "@id": "#collection",
            "@type": "RepositoryCollection"
        }
```

In the following case, on the other hand, a `Dataset` will be created:

```json
        {
            "@id": "./",
            "@type": "Dataset",
            "hasPart": [{"@id": "assets/my_collection"}],
        },
        {
            "@id": "assets/my_collection",
            "@type": ["Dataset", "RepositoryCollection"]
        }
```

Patch:
diff --git a/rocrate/model/data_entity.py b/rocrate/model/data_entity.py
index d04d364..e9acbe5 100644
--- a/rocrate/model/data_entity.py
+++ b/rocrate/model/data_entity.py
@@ -24,13 +24,14 @@ from .entity import Entity
 
 class DataEntity(Entity):
 
-    def __init__(self, crate, identifier, properties=None):
-        if not identifier or str(identifier).startswith("#"):
-            raise ValueError("Identifier for data entity must be a relative path or absolute URI: %s" % identifier)
-        super(DataEntity, self).__init__(crate, identifier, properties)
-
     def filepath(self, base_path=None):
         if base_path:
             return os.path.join(base_path, self.id)
         else:
             return self.id
+
+    def write(self, base_path):
+        pass
+
+    def write_zip(self, zip_out):
+        pass
diff --git a/rocrate/rocrate.py b/rocrate/rocrate.py
index 029b8e3..b213172 100644
--- a/rocrate/rocrate.py
+++ b/rocrate/rocrate.py
@@ -32,6 +32,7 @@ from urllib.parse import urljoin
 from .model import contextentity
 from .model.entity import Entity
 from .model.root_dataset import RootDataset
+from .model.data_entity import DataEntity
 from .model.file import File
 from .model.dataset import Dataset
 from .model.metadata import Metadata, LegacyMetadata, TESTING_EXTRA_TERMS
@@ -186,42 +187,39 @@ class ROCrate():
         added_entities = []
         # iterate over data entities
         for data_entity_ref in root_entity_parts:
-            data_entity_id = data_entity_ref['@id']
-            # print(data_entity_id)
-            entity = entities[data_entity_id]
-            # basic checks should be moved to a separate function
-            if '@type' not in entity.keys():
-                raise Exception("Entity with @id:" + data_entity_id +
-                                " has no type defined")
-
-            # Data entities can have an array as @type. So far we just parse
-            # them as File class if File is in the list. For further
-            # extensions (e.g if a Workflow class is created) we can add extra
-            # cases or create a mapping table for specific combinations. See
+            id_ = data_entity_ref['@id']
+            entity = entities[id_]
+            try:
+                t = entity["@type"]
+            except KeyError:
+                raise ValueError(f'entity "{id_}" has no @type')
+            types = {_.strip() for _ in set(t if isinstance(t, list) else [t])}
+            # Deciding what to instantiate is not trivial, see
             # https://github.com/ResearchObject/ro-crate/issues/83
-            entity_types = (entity['@type']
-                            if isinstance(entity['@type'], list)
-                            else [entity['@type']])
-            if 'File' in entity_types:
+            if {'File', 'Dataset'} <= types:
+                raise ValueError("entity can't have both File and Dataset types")
+            if 'File' in types:
                 # temporary workaround, should be handled in the general case
-                cls = TestDefinition if "TestDefinition" in entity_types else File
-                identifier = entity['@id']
+                cls = TestDefinition if "TestDefinition" in types else File
                 props = {k: v for k, v in entity.items() if k != '@id'}
-                if is_url(identifier):
-                    instance = cls(self, source=identifier, properties=props)
+                if is_url(id_):
+                    instance = cls(self, source=id_, properties=props)
                 else:
                     instance = cls(
                         self,
-                        source=os.path.join(source, identifier),
-                        dest_path=identifier,
+                        source=os.path.join(source, id_),
+                        dest_path=id_,
                         properties=props
                     )
-            if 'Dataset' in entity_types:
-                dir_path = os.path.join(source, entity['@id'])
+            elif 'Dataset' in types:
+                dir_path = os.path.join(source, id_)
                 props = {k: v for k, v in entity.items() if k != '@id'}
-                instance = Dataset(self, dir_path, entity['@id'], props)
+                instance = Dataset(self, dir_path, id_, props)
+            else:
+                props = {k: v for k, v in entity.items() if k != '@id'}
+                instance = DataEntity(self, identifier=id_, properties=props)
             self.add(instance)
-            added_entities.append(data_entity_id)
+            added_entities.append(id_)
 
         # the rest of the entities must be contextual entities
         prebuilt_entities = [


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index aaf5169..e9d211c 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -20,7 +20,9 @@ import uuid
 
 import pytest
 from rocrate.rocrate import ROCrate
+from rocrate.model.data_entity import DataEntity
 from rocrate.model.file import File
+from rocrate.model.dataset import Dataset
 from rocrate.model.computationalworkflow import ComputationalWorkflow
 from rocrate.model.person import Person
 from rocrate.model.preview import Preview
@@ -70,6 +72,16 @@ def test_dereferencing_equivalent_id(test_data_dir, name):
         assert deref_entity is entity
 
 
+def test_data_entities(test_data_dir):
+    crate = ROCrate()
+    file_ = crate.add(File(crate, test_data_dir / 'sample_file.txt'))
+    dataset = crate.add(Dataset(crate, test_data_dir / 'test_add_dir'))
+    data_entity = crate.add(DataEntity(crate, '#mysterious'))
+    assert set(crate.data_entities) == {file_, dataset, data_entity}
+    part_ids = set(_["@id"] for _ in crate.root_dataset._jsonld["hasPart"])
+    assert set(_.id for _ in (file_, dataset, data_entity)) <= part_ids
+
+
 def test_contextual_entities():
     crate = ROCrate()
     new_person = crate.add(Person(crate, '#joe', {'name': 'Joe Pesci'}))
diff --git a/test/test_read.py b/test/test_read.py
index e170040..67f19b1 100644
--- a/test/test_read.py
+++ b/test/test_read.py
@@ -15,6 +15,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import json
 import pytest
 import shutil
 import uuid
@@ -22,6 +23,7 @@ import zipfile
 from pathlib import Path
 
 from rocrate.rocrate import ROCrate
+from rocrate.model.data_entity import DataEntity
 from rocrate.model.file import File
 from rocrate.model.dataset import Dataset
 
@@ -298,3 +300,58 @@ def test_missing_file(test_data_dir, tmpdir):
     out_path = tmpdir / 'crate_read_out'
     crate.write_crate(out_path)
     assert not (out_path / name).exists()
+
+
+def test_generic_data_entity(tmpdir):
+    rc_id = "#collection"
+    metadata = {
+        "@context": [
+            "https://w3id.org/ro/crate/1.1/context",
+            {"@vocab": "http://schema.org/"},
+            {"@base": None}
+        ],
+        "@graph": [
+            {
+                "@id": "ro-crate-metadata.json",
+                "@type": "CreativeWork",
+                "about": {
+                    "@id": "./"
+                },
+                "identifier": "ro-crate-metadata.json"
+            },
+            {
+                "@id": "./",
+                "@type": "Dataset",
+                "hasPart": [{"@id": rc_id}],
+                "name": "Test RepositoryCollection"
+            },
+            {
+                "@id": rc_id,
+                "@type": "RepositoryCollection",
+                "name": "Test collection"
+            }
+        ]
+    }
+    crate_dir = tmpdir / "test_repository_collection"
+    crate_dir.mkdir()
+    with open(crate_dir / "ro-crate-metadata.json", "wt") as f:
+        json.dump(metadata, f, indent=4)
+    crate = ROCrate(crate_dir)
+
+    def check_rc():
+        rc = crate.dereference(rc_id)
+        assert rc is not None
+        assert isinstance(rc, DataEntity)
+        assert rc.id == rc_id
+        assert rc.type == "RepositoryCollection"
+        assert rc._jsonld["name"] == "Test collection"
+        assert crate.data_entities == [rc]
+        assert not crate.contextual_entities
+
+    check_rc()
+
+    out_crate_dir = tmpdir / "output_crate"
+    crate.write_crate(out_crate_dir)
+    crate = ROCrate(out_crate_dir)
+
+    check_rc()