Merge pull request ResearchObject#204 from simleo/dont_auto_add_hash

Do not auto-preprend a hash to contextual entity ids

README.md

@@ -404,9 +404,9 @@ Now the workflow has a type of `["File", "SoftwareSourceCode", "ComputationalWor
 To add [workflow testing metadata](https://crs4.github.io/life_monitor/workflow_testing_ro_crate) to the crate:
 
 ```bash
-rocrate add test-suite -i test1
-rocrate add test-instance test1 http://example.com -r jobs -i test1_1
-rocrate add test-definition test1 test/test1/sort-and-change-case-test.yml -e planemo -v '>=0.70'
+rocrate add test-suite -i '#test1'
+rocrate add test-instance '#test1' http://example.com -r jobs -i '#test1_1'
+rocrate add test-definition '#test1' test/test1/sort-and-change-case-test.yml -e planemo -v '>=0.70'
 ```
 
 To add files or directories after crate initialization:

rocrate/cli.py

@@ -25,7 +25,6 @@
 from .model.computerlanguage import LANG_MAP
 from .model.testservice import SERVICE_MAP
 from .model.softwareapplication import APP_MAP
-from .model.contextentity import add_hash
 
 
 LANG_CHOICES = list(LANG_MAP)
@@ -175,7 +174,7 @@ def workflow(crate_dir, path, language, property):
 def suite(crate_dir, identifier, name, main_entity, property):
     crate = ROCrate(crate_dir, init=False, gen_preview=False)
     suite = crate.add_test_suite(
-        identifier=add_hash(identifier),
+        identifier=identifier,
         name=name,
         main_entity=main_entity,
         properties=dict(property),
@@ -196,11 +195,11 @@ def suite(crate_dir, identifier, name, main_entity, property):
 def instance(crate_dir, suite, url, resource, service, identifier, name, property):
     crate = ROCrate(crate_dir, init=False, gen_preview=False)
     instance_ = crate.add_test_instance(
-        add_hash(suite),
+        suite,
         url,
         resource=resource,
         service=service,
-        identifier=add_hash(identifier),
+        identifier=identifier,
         name=name,
         properties=dict(property),
     )
@@ -224,7 +223,7 @@ def definition(crate_dir, suite, path, engine, engine_version, property):
         # 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(
-        add_hash(suite),
+        suite,
         source=source,
         dest_path=dest_path,
         engine=engine,

rocrate/model/contextentity.py

@@ -19,20 +19,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from ..utils import is_url
 from .entity import Entity
 
 
-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):
-        return add_hash(identifier)
+    pass

test/test_cli.py

@@ -279,8 +279,7 @@ def test_cli_add_test_metadata(test_data_dir, helpers, monkeypatch, cwd):
     assert set(TESTING_EXTRA_TERMS.items()).issubset(extra_terms.items())
 
 
-@pytest.mark.parametrize("hash_", [False, True])
-def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch, hash_):
+def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch):
     # init
     crate_dir = test_data_dir / "ro-crate-galaxy-sortchangecase"
     runner = CliRunner()
@@ -290,17 +289,15 @@ def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch,
     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])
+    result = runner.invoke(cli, ["add", "test-suite", "-c", str(crate_dir), "-i", suite_id])
     assert result.exit_code == 0
     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 = [
-        "add", "test-instance", cli_suite_id, "http://example.com", "-c", str(crate_dir), "-r", "jobs", "-i", cli_instance_id
+        "add", "test-instance", suite_id, "http://example.com", "-c", str(crate_dir), "-r", "jobs", "-i", instance_id
     ]
     result = runner.invoke(cli, args)
     assert result.exit_code == 0
@@ -310,7 +307,7 @@ def test_cli_add_test_metadata_explicit_ids(test_data_dir, helpers, monkeypatch,
     # 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)]
+    args = ["add", "test-definition", "-c", str(crate_dir), "-e", "planemo", "-v", ">=0.70", suite_id, str(def_path)]
     result = runner.invoke(cli, args)
     assert result.exit_code == 0
     json_entities = helpers.read_json_entities(crate_dir)

test/test_model.py

@@ -155,32 +155,6 @@ 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
-    email = "[email protected]"
-    email_uri = f"mailto:{email}"
-    contact_point = crate.add(ContextEntity(crate, email_uri, properties={
-        "@type": "ContactPoint",
-        "email": email
-    }))
-    crate.root_dataset["contactPoint"] = contact_point
-    assert contact_point.id == email_uri
-
-
 def test_properties():
     crate = ROCrate()
 

test/test_test_metadata.py

@@ -159,20 +159,20 @@ def test_add_test_suite(test_data_dir):
     assert s1["mainEntity"] is wf
     suites.add(s1)
     assert suites == set(crate.test_suites)
-    s2 = crate.add_test_suite(identifier="test1")
+    s2 = crate.add_test_suite(identifier="#test1")
     assert s2["mainEntity"] is wf
     assert s2.id == "#test1"
     suites.add(s2)
     assert suites == set(crate.test_suites)
-    s3 = crate.add_test_suite(identifier="test2", name="Test 2")
+    s3 = crate.add_test_suite(identifier="#test2", name="Test 2")
     assert s3["mainEntity"] is wf
     assert s3.id == "#test2"
     assert s3.name == "Test 2"
     suites.add(s3)
     assert suites == set(crate.test_suites)
     wf2_path = top_dir / "README.md"
     wf2 = crate.add(ComputationalWorkflow(crate, wf2_path, wf2_path.name))
-    s4 = crate.add_test_suite(identifier="test3", name="Foo", main_entity=wf2)
+    s4 = crate.add_test_suite(identifier="#test3", name="Foo", main_entity=wf2)
     assert s4["mainEntity"] is wf2
     assert s4.id == "#test3"
     assert s4.name == "Foo"
@@ -215,7 +215,7 @@ def test_add_test_instance(test_data_dir):
     assert i4.service is crate.dereference(TRAVIS)
     instances.add(i4)
     assert instances == set(suite.instance)
-    i5 = crate.add_test_instance(suite, "http://example.com", identifier="test_1_1")
+    i5 = crate.add_test_instance(suite, "http://example.com", identifier="#test_1_1")
     assert i5.url == "http://example.com"
     assert i5.id == "#test_1_1"
     instances.add(i5)

test/test_write.py

@@ -36,7 +36,7 @@ def test_file_writing(test_data_dir, tmpdir, helpers, gen_preview, to_zip):
     crate = ROCrate(gen_preview=gen_preview)
     crate_name = 'Test crate'
     crate.name = crate_name
-    creator_id = '001'
+    creator_id = '#001'
     creator_name = 'Lee Ritenour'
     new_person = Person(crate, creator_id, {'name': creator_name})
     crate.add(new_person)
@@ -93,10 +93,9 @@ def test_file_writing(test_data_dir, tmpdir, helpers, gen_preview, to_zip):
     root = json_entities["./"]
     assert root["name"] == crate_name
     assert "datePublished" in root
-    formatted_creator_id = f"#{creator_id}"
-    assert root["creator"] == {"@id": formatted_creator_id}
-    assert formatted_creator_id in json_entities
-    assert json_entities[formatted_creator_id]["name"] == creator_name
+    assert root["creator"] == {"@id": creator_id}
+    assert creator_id in json_entities
+    assert json_entities[creator_id]["name"] == creator_name
     if gen_preview:
         assert helpers.PREVIEW_FILE_NAME in json_entities
     file_entity = json_entities[sample_file_id]