Repository: ResearchObject/ro-crate-py
Pull request #120 (2022-05-02 08:12:40)

Fix handling of hash marks in IDs
* Completes the fix for #107 (#111 did not take into account the specification of the suite's `@id` in `add_test_definition`)
* Fixes a problem in the method that automatically adds a leading hash mark to contextual entity IDs (see https://github.com/ResearchObject/ro-crate/issues/200#issuecomment-1112649343)


Patch:
diff --git a/rocrate/cli.py b/rocrate/cli.py
index 6463223..d751581 100644
--- a/rocrate/cli.py
+++ b/rocrate/cli.py
@@ -24,7 +24,7 @@ from .rocrate import ROCrate
 from .model.computerlanguage import LANG_MAP
 from .model.testservice import SERVICE_MAP
 from .model.softwareapplication import APP_MAP
-from .utils import is_url
+from .model.contextentity import add_hash
 
 
 LANG_CHOICES = list(LANG_MAP)
@@ -32,12 +32,6 @@ SERVICE_CHOICES = list(SERVICE_MAP)
 ENGINE_CHOICES = list(APP_MAP)
 
 
-def add_hash(id_):
-    if id_ is None or id_.startswith("#") or is_url(id_):
-        return id_
-    return "#" + id_
-
-
 class State:
     pass
 
@@ -140,7 +134,10 @@ def definition(crate_dir, suite, path, engine, engine_version):
     except ValueError:
         # For now, only support marking an existing file as a test definition
         raise ValueError(f"{source} is not in the crate dir {crate_dir}")
-    crate.add_test_definition(suite, source=source, dest_path=dest_path, engine=engine, engine_version=engine_version)
+    crate.add_test_definition(
+        add_hash(suite), source=source, dest_path=dest_path, engine=engine,
+        engine_version=engine_version
+    )
     crate.metadata.write(crate_dir)
 
 
diff --git a/rocrate/model/contextentity.py b/rocrate/model/contextentity.py
index e0a5cfc..7e4a659 100644
--- a/rocrate/model/contextentity.py
+++ b/rocrate/model/contextentity.py
@@ -64,18 +64,19 @@ The corresponding plural setter supports any iterable (e.g. list):
 """
 
 
+def add_hash(id_):
+    if id_ is None or "#" in id_ or is_url(id_):
+        return id_
+    return "#" + id_
+
+
 class ContextEntity(Entity):
 
     def __init__(self, crate, identifier=None, properties=None):
         super(ContextEntity, self).__init__(crate, identifier, properties)
 
     def format_id(self, identifier):
-        if is_url(identifier):
-            return identifier
-        elif identifier.startswith('#'):
-            return identifier
-        else:
-            return '#' + identifier
+        return add_hash(identifier)
 
     def getmany(self, instance):
         for json in as_list(instance.get(self.property)):


Test patch:
diff --git a/test/test_cli.py b/test/test_cli.py
index d334a79..1f2be2d 100644
--- a/test/test_cli.py
+++ b/test/test_cli.py
@@ -214,11 +214,14 @@ def test_cli_add_test_metadata(test_data_dir, helpers, monkeypatch, cwd):
 
 @pytest.mark.parametrize("hash_", [False, True])
 def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch, hash_):
+    # init
     crate_dir = test_data_dir / "ro-crate-galaxy-sortchangecase"
     runner = CliRunner()
     assert runner.invoke(cli, ["init", "-c", str(crate_dir)]).exit_code == 0
+    # add workflow
     wf_path = crate_dir / "sort-and-change-case.ga"
     assert runner.invoke(cli, ["add", "workflow", "-c", str(crate_dir), "-l", "galaxy", str(wf_path)]).exit_code == 0
+    # add test suite
     suite_id = "#foo"
     cli_suite_id = suite_id if hash_ else suite_id[1:]
     result = runner.invoke(cli, ["add", "test-suite", "-c", str(crate_dir), "-i", cli_suite_id])
@@ -226,6 +229,7 @@ def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch,
     assert result.output.strip() == suite_id
     json_entities = helpers.read_json_entities(crate_dir)
     assert suite_id in json_entities
+    # add test instance
     instance_id = "#bar"
     cli_instance_id = instance_id if hash_ else instance_id[1:]
     args = [
@@ -236,6 +240,15 @@ def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch,
     assert result.output.strip() == instance_id
     json_entities = helpers.read_json_entities(crate_dir)
     assert instance_id in json_entities
+    # add test definition
+    def_id = "test/test1/sort-and-change-case-test.yml"
+    def_path = crate_dir / def_id
+    args = ["add", "test-definition", "-c", str(crate_dir), "-e", "planemo", "-v", ">=0.70", cli_suite_id, str(def_path)]
+    result = runner.invoke(cli, args)
+    assert result.exit_code == 0
+    json_entities = helpers.read_json_entities(crate_dir)
+    assert def_id in json_entities
+    assert set(json_entities[def_id]["@type"]) == {"File", "TestDefinition"}
 
 
 @pytest.mark.parametrize("cwd", [False, True])
diff --git a/test/test_model.py b/test/test_model.py
index 9c48b0c..3bdc539 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -30,6 +30,7 @@ from rocrate.model.dataset import Dataset
 from rocrate.model.computationalworkflow import ComputationalWorkflow
 from rocrate.model.person import Person
 from rocrate.model.preview import Preview
+from rocrate.model.contextentity import ContextEntity
 
 
 RAW_REPO_URL = "https://raw.githubusercontent.com/ResearchObject/ro-crate-py"
@@ -134,6 +135,24 @@ def test_contextual_entities():
     assert person_dereference is new_person
 
 
+def test_contextual_entities_hash(test_data_dir):
+    crate = ROCrate()
+    john = crate.add(Person(crate, "john", properties={"name": "John Doe"}))
+    assert john.id == "#john"
+    id_ = "https://orcid.org/0000-0002-1825-0097"
+    josiah = crate.add(Person(crate, id_, properties={"name": "Josiah Carberry"}))
+    assert josiah.id == id_
+    wf_path = test_data_dir / "ro-crate-galaxy-sortchangecase" / "sort-and-change-case.ga"
+    wf_dest_path = wf_path.name
+    wf = crate.add_workflow(wf_path, wf_dest_path, main=True, lang="galaxy")
+    step_id = f"{wf_dest_path}#sort"
+    step = crate.add(ContextEntity(crate, step_id, properties={
+        "@type": "HowToStep",
+    }))
+    wf["hasPart"] = [step]
+    assert step.id == step_id
+
+
 def test_properties():
     crate = ROCrate()