Repository: ResearchObject/ro-crate-py
Pull request #88 (2021-11-09 17:30:51)

Created check for updating an existing ro-crate and accompanying test…
Created a PR based on this issue https://github.com/ResearchObject/ro-crate-py/issues/86


Patch:
diff --git a/rocrate/model/computationalworkflow.py b/rocrate/model/computationalworkflow.py
index fcb3d25..ab5a943 100644
--- a/rocrate/model/computationalworkflow.py
+++ b/rocrate/model/computationalworkflow.py
@@ -42,7 +42,7 @@ class ComputationalWorkflow(File):
 
     @property
     def programmingLanguage(self):
-        return self.get("programmingLanguage")
+        return self["programmingLanguage"]
 
     @programmingLanguage.setter
     def programmingLanguage(self, programmingLanguage):
@@ -52,7 +52,7 @@ class ComputationalWorkflow(File):
 
     @property
     def subjectOf(self):
-        return self.get("subjectOf")
+        return self["subjectOf"]
 
     @subjectOf.setter
     def subjectOf(self, subjectOf):
diff --git a/rocrate/model/computerlanguage.py b/rocrate/model/computerlanguage.py
index 6571cd9..8b9a524 100644
--- a/rocrate/model/computerlanguage.py
+++ b/rocrate/model/computerlanguage.py
@@ -28,7 +28,7 @@ class ComputerLanguage(ContextEntity):
 
     @property
     def name(self):
-        return self.get("name")
+        return self["name"]
 
     @name.setter
     def name(self, name):
@@ -36,7 +36,7 @@ class ComputerLanguage(ContextEntity):
 
     @property
     def alternateName(self):
-        return self.get("alternateName")
+        return self["alternateName"]
 
     @alternateName.setter
     def alternateName(self, alternateName):
@@ -44,7 +44,7 @@ class ComputerLanguage(ContextEntity):
 
     @property
     def identifier(self):
-        return self.get("identifier")
+        return self["identifier"]
 
     @identifier.setter
     def identifier(self, identifier):
@@ -52,7 +52,7 @@ class ComputerLanguage(ContextEntity):
 
     @property
     def url(self):
-        return self.get("url")
+        return self["url"]
 
     @url.setter
     def url(self, url):
@@ -61,7 +61,7 @@ class ComputerLanguage(ContextEntity):
     # Not listed as a property in "https://schema.org/ComputerLanguage"
     @property
     def version(self):
-        return self.get("version")
+        return self["version"]
 
     @version.setter
     def version(self, version):
diff --git a/rocrate/model/entity.py b/rocrate/model/entity.py
index 826b564..6b29bd1 100644
--- a/rocrate/model/entity.py
+++ b/rocrate/model/entity.py
@@ -18,13 +18,12 @@
 # limitations under the License.
 
 import uuid
-from collections.abc import MutableMapping
 
 from dateutil.parser import isoparse
 from .. import vocabs
 
 
-class Entity(MutableMapping):
+class Entity(object):
 
     def __init__(self, crate, identifier=None, properties=None):
         self.crate = crate
@@ -60,11 +59,14 @@ class Entity(MutableMapping):
             return clsName
         return "Thing"
 
+    def reference(self):
+        return {'@id': self.id}
+
     def canonical_id(self):
         return self.crate.resolve_id(self.id)
 
-    def __hash__(self):
-        return hash(self.canonical_id())
+    def hash(self):
+        hash(self.canonical_id())
 
     def _empty(self):
         val = {
@@ -73,52 +75,44 @@ class Entity(MutableMapping):
         }
         return val
 
-    def __getitem__(self, key):
-        v = self._jsonld[key]
-        if isinstance(v, str) or key.startswith("@"):
-            return v
-        values = v if isinstance(v, list) else [v]
-        deref_values = [self.crate.dereference(_["@id"], _["@id"]) for _ in values]
-        return deref_values if isinstance(v, list) else deref_values[0]
+    def auto_dereference(self, value):
+        if isinstance(value, list):
+            return_list = []
+            for entry in value:
+                return_list.append(self.auto_dereference(entry))
+            return return_list
+        if isinstance(value, dict) and value['@id']:  # its a reference
+            obj = self.crate.dereference(value['@id'])
+            return obj if obj else value['@id']
+        return value
+
+    def auto_reference(self, value):
+        if isinstance(value, list):  # TODO: make it in a more pythonic way
+            return_list = []
+            for entry in value:
+                return_list.append(self.auto_reference(entry))
+            return return_list
+        if isinstance(value, Entity):
+            # add reference to an Entity
+            return value.reference()  # I assume it is already in the crate...
+        else:
+            return value
+
+    def __getitem__(self, key: str):
+        if key in self._jsonld.keys():
+            return self.auto_dereference(self._jsonld[key])
+        else:
+            return None
 
     def __setitem__(self, key: str, value):
-        if key.startswith("@"):
-            raise KeyError(f"cannot set '{key}'")
-        values = value if isinstance(value, list) else [value]
-        ref_values = [{"@id": _.id} if isinstance(_, Entity) else _ for _ in values]
-        self._jsonld[key] = ref_values if isinstance(value, list) else ref_values[0]
+        self._jsonld[key] = self.auto_reference(value)
 
     def __delitem__(self, key: str):
-        if key.startswith("@"):
-            raise KeyError(f"cannot delete '{key}'")
         del self._jsonld[key]
 
-    def popitem(self):
-        raise NotImplementedError
-
-    def clear(self):
-        raise NotImplementedError
-
-    def update(self):
-        raise NotImplementedError
-
-    def __iter__(self):
-        return iter(self._jsonld)
-
-    def __len__(self):
-        return len(self._jsonld)
-
-    def __contains__(self, key):
-        return key in self._jsonld
-
-    def __eq__(self, other):
-        if not isinstance(other, Entity):
-            return NotImplemented
-        return self.id == other.id and self._jsonld == other._jsonld
-
     @property
-    def type(self):
-        return self._jsonld['@type']
+    def type(self) -> str:
+        return self['@type']
 
     # @property
     # def types(self)-> List[str]:
@@ -126,7 +120,7 @@ class Entity(MutableMapping):
 
     @property
     def datePublished(self):
-        d = self.get('datePublished')
+        d = self['datePublished']
         return d if not d else isoparse(d)
 
     @datePublished.setter
diff --git a/rocrate/model/file.py b/rocrate/model/file.py
index 7a857a5..dc5ddd1 100644
--- a/rocrate/model/file.py
+++ b/rocrate/model/file.py
@@ -58,4 +58,5 @@ class File(FileOrDir):
                         shutil.copyfileobj(response, out_file)
         elif os.path.isfile(self.source):
             out_file_path.parent.mkdir(parents=True, exist_ok=True)
-            shutil.copy(self.source, out_file_path)
+            if not out_file_path.exists() or not out_file_path.samefile(self.source):
+                shutil.copy(self.source, out_file_path)
diff --git a/rocrate/model/softwareapplication.py b/rocrate/model/softwareapplication.py
index 9182469..d53b6c0 100644
--- a/rocrate/model/softwareapplication.py
+++ b/rocrate/model/softwareapplication.py
@@ -29,7 +29,7 @@ class SoftwareApplication(ContextEntity, CreativeWork):
 
     @property
     def name(self):
-        return self.get("name")
+        return self["name"]
 
     @name.setter
     def name(self, name):
@@ -37,7 +37,7 @@ class SoftwareApplication(ContextEntity, CreativeWork):
 
     @property
     def url(self):
-        return self.get("url")
+        return self["url"]
 
     @url.setter
     def url(self, url):
@@ -45,7 +45,7 @@ class SoftwareApplication(ContextEntity, CreativeWork):
 
     @property
     def version(self):
-        return self.get("version")
+        return self["version"]
 
     @version.setter
     def version(self, version):
diff --git a/rocrate/rocrate.py b/rocrate/rocrate.py
index 31df59f..aec9987 100644
--- a/rocrate/rocrate.py
+++ b/rocrate/rocrate.py
@@ -219,7 +219,7 @@ class ROCrate():
 
     @property
     def name(self):
-        return self.root_dataset.get('name')
+        return self.root_dataset['name']
 
     @name.setter
     def name(self, value):
@@ -235,7 +235,7 @@ class ROCrate():
 
     @property
     def creator(self):
-        return self.root_dataset.get('creator')
+        return self.root_dataset['creator']
 
     @creator.setter
     def creator(self, value):
@@ -243,7 +243,7 @@ class ROCrate():
 
     @property
     def license(self):
-        return self.root_dataset.get('license')
+        return self.root_dataset['license']
 
     @license.setter
     def license(self, value):
@@ -251,7 +251,7 @@ class ROCrate():
 
     @property
     def description(self):
-        return self.root_dataset.get('description')
+        return self.root_dataset['description']
 
     @description.setter
     def description(self, value):
@@ -259,7 +259,7 @@ class ROCrate():
 
     @property
     def keywords(self):
-        return self.root_dataset.get('keywords')
+        return self.root_dataset['keywords']
 
     @keywords.setter
     def keywords(self, value):
@@ -267,7 +267,7 @@ class ROCrate():
 
     @property
     def publisher(self):
-        return self.root_dataset.get('publisher')
+        return self.root_dataset['publisher']
 
     @publisher.setter
     def publisher(self, value):
@@ -275,7 +275,7 @@ class ROCrate():
 
     @property
     def isBasedOn(self):
-        return self.root_dataset.get('isBasedOn')
+        return self.root_dataset['isBasedOn']
 
     @isBasedOn.setter
     def isBasedOn(self, value):
@@ -283,7 +283,7 @@ class ROCrate():
 
     @property
     def image(self):
-        return self.root_dataset.get('image')
+        return self.root_dataset['image']
 
     @image.setter
     def image(self, value):
@@ -291,7 +291,7 @@ class ROCrate():
 
     @property
     def CreativeWorkStatus(self):
-        return self.root_dataset.get('CreativeWorkStatus')
+        return self.root_dataset['CreativeWorkStatus']
 
     @CreativeWorkStatus.setter
     def CreativeWorkStatus(self, value):
@@ -299,7 +299,7 @@ class ROCrate():
 
     @property
     def mainEntity(self):
-        return self.root_dataset.get('mainEntity')
+        return self.root_dataset['mainEntity']
 
     @mainEntity.setter
     def mainEntity(self, value):
@@ -321,10 +321,10 @@ class ROCrate():
 
     @property
     def test_suites(self):
-        mentions = [_ for _ in self.root_dataset.get('mentions', []) if isinstance(_, TestSuite)]
-        about = [_ for _ in self.root_dataset.get('about', []) if isinstance(_, TestSuite)]
+        mentions = [_ for _ in (self.root_dataset['mentions'] or []) if isinstance(_, TestSuite)]
+        about = [_ for _ in (self.root_dataset['about'] or []) if isinstance(_, TestSuite)]
         if self.test_dir:
-            legacy_about = [_ for _ in self.test_dir.get('about', []) if isinstance(_, TestSuite)]
+            legacy_about = [_ for _ in (self.test_dir['about'] or []) if isinstance(_, TestSuite)]
             about += legacy_about
         return list(set(mentions + about))  # remove any duplicate refs
 
@@ -339,11 +339,9 @@ class ROCrate():
     def _get_root_jsonld(self):
         self.root_dataset.properties()
 
-    def dereference(self, entity_id, default=None):
+    def dereference(self, entity_id):
         canonical_id = self.resolve_id(entity_id)
-        return self.__entity_map.get(canonical_id, default)
-
-    get = dereference
+        return self.__entity_map.get(canonical_id, None)
 
     def add_file(
             self,
@@ -439,7 +437,7 @@ class ROCrate():
                     self.data_entities.remove(e)
                 except ValueError:
                     pass
-                self.root_dataset["hasPart"] = [_ for _ in self.root_dataset.get("hasPart", []) if _ != e]
+                self.root_dataset["hasPart"] = [_ for _ in self.root_dataset["hasPart"] or [] if _ != e]
                 if not self.root_dataset["hasPart"]:
                     del self.root_dataset._jsonld["hasPart"]
             else:
@@ -465,7 +463,8 @@ class ROCrate():
                 rel = source.relative_to(top)
                 if not self.dereference(str(rel)):
                     dest = base_path / rel
-                    shutil.copyfile(source, dest)
+                    if not dest.exists() or not dest.samefile(source):
+                        shutil.copyfile(source, dest)
 
     def write(self, base_path):
         base_path = Path(base_path)


Test patch:
diff --git a/rocrate/model/testdefinition.py b/rocrate/model/testdefinition.py
index 5ca6244..12be724 100644
--- a/rocrate/model/testdefinition.py
+++ b/rocrate/model/testdefinition.py
@@ -32,7 +32,7 @@ class TestDefinition(File):
 
     @property
     def engineVersion(self):
-        return self.get("engineVersion")
+        return self["engineVersion"]
 
     @engineVersion.setter
     def engineVersion(self, engineVersion):
@@ -40,7 +40,7 @@ class TestDefinition(File):
 
     @property
     def conformsTo(self):
-        return self.get("conformsTo")
+        return self["conformsTo"]
 
     @conformsTo.setter
     def conformsTo(self, conformsTo):
diff --git a/rocrate/model/testinstance.py b/rocrate/model/testinstance.py
index aac7547..754227d 100644
--- a/rocrate/model/testinstance.py
+++ b/rocrate/model/testinstance.py
@@ -32,7 +32,7 @@ class TestInstance(ContextEntity):
 
     @property
     def name(self):
-        return self.get("name")
+        return self["name"]
 
     @name.setter
     def name(self, name):
@@ -40,7 +40,7 @@ class TestInstance(ContextEntity):
 
     @property
     def resource(self):
-        return self.get("resource")
+        return self["resource"]
 
     @resource.setter
     def resource(self, resource):
@@ -48,7 +48,7 @@ class TestInstance(ContextEntity):
 
     @property
     def runsOn(self):
-        return self.get("runsOn")
+        return self["runsOn"]
 
     @runsOn.setter
     def runsOn(self, runsOn):
@@ -56,7 +56,7 @@ class TestInstance(ContextEntity):
 
     @property
     def url(self):
-        return self.get("url")
+        return self["url"]
 
     @url.setter
     def url(self, url):
diff --git a/rocrate/model/testservice.py b/rocrate/model/testservice.py
index 8ed7aca..a42b261 100644
--- a/rocrate/model/testservice.py
+++ b/rocrate/model/testservice.py
@@ -32,7 +32,7 @@ class TestService(ContextEntity):
 
     @property
     def name(self):
-        return self.get("name")
+        return self["name"]
 
     @name.setter
     def name(self, name):
@@ -40,7 +40,7 @@ class TestService(ContextEntity):
 
     @property
     def url(self):
-        return self.get("url")
+        return self["url"]
 
     @url.setter
     def url(self, url):
diff --git a/rocrate/model/testsuite.py b/rocrate/model/testsuite.py
index 1e5ad13..3edc057 100644
--- a/rocrate/model/testsuite.py
+++ b/rocrate/model/testsuite.py
@@ -32,7 +32,7 @@ class TestSuite(ContextEntity):
 
     @property
     def name(self):
-        return self.get("name")
+        return self["name"]
 
     @name.setter
     def name(self, name):
@@ -40,7 +40,7 @@ class TestSuite(ContextEntity):
 
     @property
     def instance(self):
-        return self.get("instance")
+        return self["instance"]
 
     @instance.setter
     def instance(self, instance):
@@ -48,7 +48,7 @@ class TestSuite(ContextEntity):
 
     @property
     def definition(self):
-        return self.get("definition")
+        return self["definition"]
 
     @definition.setter
     def definition(self, definition):
diff --git a/test/test_api.py b/test/test_api.py
index db2c20f..8764981 100644
--- a/test/test_api.py
+++ b/test/test_api.py
@@ -32,8 +32,8 @@ def test_galaxy_wf_crate(test_data_dir, tmpdir, helpers):
     lang = wf_crate.dereference("#galaxy")
     assert hasattr(lang, "name")
     assert lang.version == GALAXY_DEFAULT_VERSION
-    assert wf.get("programmingLanguage") is lang
-    assert wf.get("subjectOf") is not None
+    assert wf["programmingLanguage"] is lang
+    assert wf["subjectOf"] is not None
     assert helpers.WORKFLOW_TYPES.issubset(wf["subjectOf"].type)
 
     out_path = tmpdir / 'ro_crate_out'
@@ -67,8 +67,8 @@ def test_cwl_wf_crate(test_data_dir, tmpdir, helpers):
     lang = wf_crate.dereference("#cwl")
     assert hasattr(lang, "name")
     assert lang.version == CWL_DEFAULT_VERSION
-    assert wf.get("programmingLanguage") is lang
-    assert "subjectOf" not in wf
+    assert wf["programmingLanguage"] is lang
+    assert wf["subjectOf"] is None
 
     out_path = tmpdir / 'ro_crate_out'
     out_path.mkdir()
@@ -98,8 +98,8 @@ def test_create_wf_include(test_data_dir, tmpdir, helpers):
     lang = wf_crate.dereference("#galaxy")
     assert hasattr(lang, "name")
     assert lang.version == GALAXY_DEFAULT_VERSION
-    assert wf.get("programmingLanguage") is lang
-    assert wf.get("subjectOf") is not None
+    assert wf["programmingLanguage"] is lang
+    assert wf["subjectOf"] is not None
     assert helpers.WORKFLOW_TYPES.issubset(wf["subjectOf"].type)
     assert wf_crate.dereference(extra_file1.name) is not None
     assert wf_crate.dereference(extra_file2.name) is not None
diff --git a/test/test_model.py b/test/test_model.py
index 57fad30..96ba479 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -16,7 +16,6 @@
 # limitations under the License.
 
 import datetime
-import json
 import tempfile
 import uuid
 from pathlib import Path
@@ -55,13 +54,6 @@ def test_dereferencing(test_data_dir, helpers):
     readme_entity = crate.add_file(readme_url)
     assert crate.dereference(readme_url) is readme_entity
 
-    # dereference nonexistent entity
-    assert crate.dereference(f"#{uuid.uuid4()}") is None
-    assert crate.dereference(f"#{uuid.uuid4()}", file_returned) is file_returned
-
-    # alias
-    assert crate.get(readme_url) is readme_entity
-
 
 @pytest.mark.parametrize("name", [".foo", "foo.", ".foo/", "foo./"])
 def test_dereferencing_equivalent_id(test_data_dir, name):
@@ -178,7 +170,7 @@ def test_uuid():
 
 def test_update(test_data_dir, tmpdir, helpers):
     crate = ROCrate()
-    assert "hasPart" not in crate.root_dataset
+    assert not crate.root_dataset["hasPart"]
     wf_path = test_data_dir / "test_galaxy_wf.ga"
     john = crate.add(Person(crate, '#john', {'name': 'John Doe'}))
     file_ = crate.add_file(wf_path)
@@ -188,7 +180,7 @@ def test_update(test_data_dir, tmpdir, helpers):
     assert crate.mainEntity is None
     wf = crate.add_workflow(wf_path, main=True, lang="galaxy")
     assert isinstance(wf, ComputationalWorkflow)
-    assert "author" not in wf
+    assert wf["author"] is None
     assert crate.mainEntity is wf
     assert crate.dereference(john.id) is john
     assert crate.dereference(file_.id) is wf
@@ -226,7 +218,7 @@ def test_delete(test_data_dir):
     assert file1 not in crate.root_dataset["hasPart"]
     crate.delete(file2)
     assert file2 not in crate.data_entities
-    assert "hasPart" not in crate.root_dataset
+    assert crate.root_dataset["hasPart"] is None
     crate.delete(file2)  # no-op
     # contextual entities
     john = crate.add(Person(crate, '#john', {'name': 'John Doe'}))
@@ -267,7 +259,7 @@ def test_delete_by_id(test_data_dir):
     assert f.id == path.name
     crate.delete(path.name)
     assert f not in crate.data_entities
-    assert "hasPart" not in crate.root_dataset
+    assert crate.root_dataset["hasPart"] is None
 
 
 def test_self_delete(test_data_dir):
@@ -278,56 +270,4 @@ def test_self_delete(test_data_dir):
     assert f in crate.root_dataset["hasPart"]
     f.delete()
     assert f not in crate.data_entities
-    assert "hasPart" not in crate.root_dataset
-
-
-def test_entity_as_mapping(tmpdir, helpers):
-    orcid = "https://orcid.org/0000-0002-1825-0097"
-    metadata = {
-        "@context": "https://w3id.org/ro/crate/1.1/context",
-        "@graph": [
-            {"@id": "ro-crate-metadata.json",
-             "@type": "CreativeWork",
-             "about": {"@id": "./"},
-             "conformsTo": {"@id": "https://w3id.org/ro/crate/1.1"}},
-            {"@id": "./",
-             "@type": "Dataset",
-             "author": {"@id": orcid}},
-            {"@id": orcid,
-             "@type": "Person",
-             "givenName": "Josiah",
-             "familyName": "Carberry"}
-        ]
-    }
-    crate_dir = tmpdir / "in_crate"
-    crate_dir.mkdir()
-    with open(crate_dir / helpers.METADATA_FILE_NAME, "wt") as f:
-        json.dump(metadata, f, indent=4)
-    crate = ROCrate(crate_dir)
-    person = crate.dereference(orcid)
-    assert len(person) == 4
-    assert len(list(person)) == 4
-    assert set(person) == set(person.keys()) == {"@id", "@type", "givenName", "familyName"}
-    assert set(person.values()) == {orcid, "Person", "Josiah", "Carberry"}
-    assert set(person.items()) == set(zip(person.keys(), person.values()))
-    assert person.setdefault("givenName", "foo") == "Josiah"
-    assert len(person) == 4
-    assert person.setdefault("award", "Oscar") == "Oscar"
-    assert len(person) == 5
-    assert "award" in person
-    assert person.pop("award") == "Oscar"
-    assert len(person) == 4
-    assert "award" not in person
-    for key in "@id", "@type":
-        with pytest.raises(KeyError):
-            person[key] = "foo"
-        with pytest.raises(KeyError):
-            del person[key]
-        with pytest.raises(KeyError):
-            person.pop(key)
-    twin = Person(crate, orcid, properties={
-        "givenName": "Josiah",
-        "familyName": "Carberry"
-    })
-    assert twin == person
-    assert Person(crate, orcid) != person
+    assert crate.root_dataset["hasPart"] is None
diff --git a/test/test_read.py b/test/test_read.py
index 8db4036..d8be880 100644
--- a/test/test_read.py
+++ b/test/test_read.py
@@ -245,7 +245,7 @@ def test_no_parts(tmpdir):
     crate.write(out_path)
 
     crate = ROCrate(out_path)
-    assert "hasPart" not in crate.root_dataset
+    assert not crate.root_dataset["hasPart"]
 
 
 @pytest.mark.parametrize("to_zip", [False, True])
diff --git a/test/test_readwrite.py b/test/test_readwrite.py
new file mode 100644
index 0000000..a29b0b2
--- /dev/null
+++ b/test/test_readwrite.py
@@ -0,0 +1,37 @@
+# Copyright 2019-2021 The University of Manchester, UK
+# Copyright 2020-2021 Vlaams Instituut voor Biotechnologie (VIB), BE
+# Copyright 2020-2021 Barcelona Supercomputing Center (BSC), ES
+# Copyright 2020-2021 Center for Advanced Studies, Research and Development in Sardinia (CRS4), IT
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+from rocrate.rocrate import ROCrate
+
+
+def test_file_rewriting(test_data_dir, helpers):
+    # load crate
+    crate_dir = test_data_dir / 'read_crate'
+    crate = ROCrate(crate_dir)
+
+    crate.write(crate_dir)
+
+    metadata_path = crate_dir / helpers.METADATA_FILE_NAME
+    assert metadata_path.exists()
+
+    sample_file_id = "test_file_galaxy.txt"
+    sample_file2_id = "test_galaxy_wf.ga"
+    file1 = crate_dir / sample_file_id
+    file2 = crate_dir / sample_file2_id
+    assert file1.exists()
+    assert file2.exists()