-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --copy-annotation option to ontoconvert #732
Changes from 3 commits
b57d5b3
ac3dd97
e6b6348
59f8b4e
49609a3
d861923
4ea6ce8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -835,3 +835,34 @@ | |
) | ||
|
||
return layout | ||
|
||
|
||
def copy_annotation(onto, src, dst): | ||
"""In all classes and properties in `onto`, copy annotation `src` to `dst`. | ||
|
||
The `src` and `dst` can either be provided as a label string or a full IRI. | ||
""" | ||
if onto.world[src]: | ||
src = onto.world[src] | ||
Comment on lines
+849
to
+850
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why check in world? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we want to support src of the form |
||
else: | ||
src = onto[src] | ||
|
||
if onto.world[dst]: | ||
dst = onto.world[dst] | ||
elif dst in onto: | ||
dst = onto[dst] | ||
else: | ||
if "://" not in dst: | ||
raise ValueError( | ||
Comment on lines
+859
to
+860
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should have tests for errors that we expect may happen. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this such a test? True, there might exists invalid IRIs that contain "://", but this easy and covers most of all error cases. The rest will raise an exception from owlready2 when trying to create a new annotation property. What other errors would you expect? |
||
"new destination annotation property must be provided as " | ||
"a full IRI" | ||
) | ||
name = min(dst.rsplit("#")[-1], dst.rsplit("/")[-1]) | ||
iri = dst | ||
dst = onto.new_annotation_property(name, owlready2.AnnotationProperty) | ||
dst.iri = iri | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already tested in tests/tools/test_ontoconvert.py Just codecov that doesn't recognise tests done in subprocesses... |
||
|
||
for e in onto.get_entities(): | ||
new = getattr(e, src.name).first() | ||
if new and new not in getattr(e, dst.name): | ||
getattr(e, dst.name).append(new) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,3 +39,18 @@ def test_run() -> None: | |
assert re.search("@prefix : <https://w3id.org/ex/testonto#>", output2) | ||
assert re.search("<https://w3id.org/ex/testonto> .* owl:Ontology", output2) | ||
assert re.search("testclass .* owl:Class", output2) | ||
|
||
# Test 3 - copy-annotation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are checking copy_preflabel. I agree that this is an alias for annotation, but I think it would be good to check that other annotations also are added. Most particularly, that we can copy to an annotation that is originally not in the ontology. Also, I think we should have a test where preflabel is not converted, as prefLabel is copied by default (if I understand store_true correctly). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rdfs:label is not in the ontology, so we are actually creating a new annotation. Ok, added new test with |
||
ontoconvert.main( | ||
[ | ||
"-p", | ||
"--iri=https://w3id.org/ex/testonto", | ||
"--base-iri=https://w3id.org/ex/testonto#", | ||
str(ontodir / "testonto.ttl"), | ||
str(outdir / "test_ontoconvert3.ttl"), | ||
] | ||
) | ||
input3 = (ontodir / "testonto.ttl").read_text() | ||
output3 = (outdir / "test_ontoconvert3.ttl").read_text() | ||
assert not re.search('rdfs:label "hasAnnotationProperty"@en', input3) | ||
assert re.search('rdfs:label "hasAnnotationProperty"@en', output3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should document with "Parameters" of "Arguments", as this renders nicely in the docementaion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done