Skip to content
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

OBO parser: Treat value of replaced_by tag as an IRI #1072

Closed
wants to merge 3 commits into from

Conversation

gouttegd
Copy link
Contributor

@gouttegd gouttegd commented Aug 5, 2022

This PR is a proof-of-concept fix for the issue mentioned in INCATools/ontology-development-kit#642, i.e. the fact that the value of a replaced_by tag in a OBO file is translated into a literal string, instead of an IRI as expected.

The fix has two parts:

  • The first part is to change the way replaced_by clauses are translated by converting the value into an IRI, instead of leaving it as a string. This covers both the case where the value is already a full URL and the case where the value is a OBO CURIE, thanks to the loadIdToIRI method. This does not, however, cover the case of non-OBO CURIEs.
  • The second part makes the parser honour the idspace tags optionally found in the header frame, by feeding the specified prefix and corresponding base URL into the idSpaceMap hash map, which is already used by the loadIdToIRI method when expanding CURIEs. This covers the case of non-OBO CURIEs, by making sure they are expanded to the correct IRI instead of the default http://purl.obolibrary.org/obo/.

The value of the `replaced_by` tag in a OBO file should be an ID
according to the OBO Flat File Format specification, so we treat it as
such.
When the header frame of a OBO file contains `idspace` tags, use them to
translate Prefixed-IDs (aka CURIEs) into full IRIs.
@gouttegd gouttegd marked this pull request as draft August 5, 2022 20:41
@balhoff
Copy link
Contributor

balhoff commented Aug 5, 2022

@gouttegd this is great! I suggest handling consider the same way as replaced_by.

Process `consider` tags in a OBO file the same way as `replaced_by`
tags.
@gouttegd gouttegd marked this pull request as ready for review August 8, 2022 11:45
@cmungall
Copy link
Member

@gouttegd - does your PR fix idspace expansion for ALL CURIEs, or only CURIEs that are used in replaced_by?

@gouttegd
Copy link
Contributor Author

@cmungall For now, only CURIEs that are used in replaced_by and consider clauses.

I have another branch (not pushed anywhere yet) in which I test replacing CURIEs also in xref clauses, but I found that to be somewhat trickier. I can push that to another PR if you and others want to have a look at it.

@balhoff
Copy link
Contributor

balhoff commented Aug 19, 2022

I did some tests and found that this change also deals with term IDs using non-OBO namespaces. PRO (http://purl.obolibrary.org/obo/pr.obo) has statements like this:

idspace: HGNC http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=

And terms:

id: HGNC:100
name: ASIC1 (human)
namespace: gene
def: "A protein coding gene ASIC1 in human." [PRO:DNx]
comment: Category=external.
is_a: SO:0001217 ! protein_coding_gene
relationship: only_in_taxon NCBITaxon:9606 ! Homo sapiens

With the old code this is parsed into:

# Class: <http://purl.obolibrary.org/obo/HGNC_100> (ASIC1 (human))

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> "PRO:DNx") <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/HGNC_100> "A protein coding gene ASIC1 in human.")
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#hasOBONamespace> <http://purl.obolibrary.org/obo/HGNC_100> "gene")
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#id> <http://purl.obolibrary.org/obo/HGNC_100> "HGNC:100")
AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#comment> <http://purl.obolibrary.org/obo/HGNC_100> "Category=external.")
AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#label> <http://purl.obolibrary.org/obo/HGNC_100> "ASIC1 (human)")
SubClassOf(<http://purl.obolibrary.org/obo/HGNC_100> <http://purl.obolibrary.org/obo/SO_0001217>)
SubClassOf(<http://purl.obolibrary.org/obo/HGNC_100> ObjectSomeValuesFrom(<http://purl.obolibrary.org/obo/RO_0002160> <http://purl.obolibrary.org/obo/NCBITaxon_9606>))

With this PR it becomes:

# Class: <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> (ASIC1 (human))

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> "PRO:DNx") <http://purl.obolibrary.org/obo/IAO_0000115> <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> "A protein coding gene ASIC1 in human.")
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#hasOBONamespace> <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> "gene")
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#id> <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> "HGNC:100")
AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#comment> <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> "Category=external.")
AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#label> <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> "ASIC1 (human)")
SubClassOf(<http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> <http://purl.obolibrary.org/obo/SO_0001217>)
SubClassOf(<http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> ObjectSomeValuesFrom(<http://purl.obolibrary.org/obo/RO_0002160> <http://purl.obolibrary.org/obo/NCBITaxon_9606>))

That's really nice. How much do we want to try to handle roundtrip with these changes? If you read in that OBO file, and then write it out as OBO, you get a huge owl-axioms header containing axioms like:

AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#id> <http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100> \"HGNC:100\")

and the term stanzas look like:

[Term]
id: http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=100
name: ASIC1 (human)
namespace: gene
def: "A protein coding gene ASIC1 in human." [PRO:DNx]
comment: Category=external.
is_a: SO:0001217 ! protein_coding_gene
relationship: only_in_taxon NCBITaxon:9606 ! Homo sapiens

@shawntanzk
Copy link

Do we care about round-tripping (OBO -> OWL -> OBO results in difference)
Currently if we don't care about that, it basically works,
if not we need to figure out how to get that to work - this will take a lot more work, and we aren't sure if this is worth it?
(we cannot guarantee that we will be able to regenerate an OBO file the same as the original at the OBO file)
example of where this is an issue:
If you use a obo edit file and have a full release in owl that gets converts back into obo file -> ugly URLs instead of nice names
Note: this PR as it stands is opt-in
Ideal situation: find solution for curie round-tripping
@matentzn to give the few options

@gouttegd
Copy link
Contributor Author

To clarify:

  • As long as OBO-style CURIEs are used (CURIEs that expand to http://purl.obolibrary.org/obo/PREFIX_LOCALID), there is no problem at all: such CURIEs can be expanded when parsing a OBO file, and can be “condensed” back into proper CURIEs when writing a OBO file — because the OBO writer has heuristic in place to detect OBO-style IRIs, and knows how to make CURIEs out of them.

  • Problems arise when non-OBO CURIEs are used. This PR allows to expand them into IRIs based on prefix maps declared in idspace tags, but when converting back to OBO, the OBO writer has no way to know how to condense them into CURIEs. It will try to guess how to do it (mostly, the logic is: if the last component of the IRI is of the form AAA_BBB, then make that into a AAA:BBB CURIE), but there’s no guarantee the results will be what the authors could have expected.

So in this case, “round-tripping” a OBO file (converting from OBO to any other format and then back to OBO again) can and most likely will produce a different OBO file, where IRIs are either condensed into different CURIEs than the ones in the original file, or not condensed at all.

  • This would only happen if the original OBO file contains idspace tags for prefixes that should be mapped to non-OBO CURIEs. Without such tags, all prefixes are assumed to be OBO prefixes, so all CURIEs will be expanded to OBO-style IRIs. And because the OBO writer knows how to condense OBO-style IRIs, this ensures a “round-tripped“ OBO file will always contain the same CURIEs as the original file.

This is the “opt-in” bit mentioned by @shawntanzk above: Non-OBO CURIEs are only expanded to non-OBO IRIs if the editors explicitly asked for it by using a idspace. If such an expansion is unwanted, don’t use idspace. This is a slight extension of the meaning of the idspace tag (from “this prefix is mapped to this IRI scheme” to “this prefix is mapped to this IRI scheme – and by saying so, I am hereby asking the parser to expand any instance of this prefix”), but I believe it makes sense.

  • So the question is: if/when we expand non-OBO CURIEs into non-OBO IRIs, should we try to make sure they can be condensed back correctly when converting again to OBO? If so, then we must somehow provide the OBO writer with the prefix maps it needs, to let it know how to perform the condensation (instead of guessing based on what the IRI looks like).

So I guess the options are:

A) Statu quo. Don’t expand CURIEs in replaced_by and similar tags, and don’t do anything with idspace tags.

B) Expand CURIEs whenever possible, but without using idspace prefix maps — assume CURIEs are OBO-style CURIEs. This will cause all CURIEs to be expanded to OBO-style IRI (http://purl.obolibrary.org/obo/PREFIX_LOCALID), which may not always be desired — but on the other hand this will ensure the expanded IRIs can always be condensed back into their original CURIEs.

C) Expand CURIEs whenever possible, using prefix maps from idspace tags when present (basically, what this PR is doing). Do not care (at least for now) about what happens to non-OBO CURIEs when writing back to OBO, just tell editors to avoid using idspace if expansion into non-OBO IRIs is not desired.

D) Expand CURIEs whenever possible, using prefix maps from idspace tags when present, and making sure the corresponding expanded IRIs can always be condensed back to their original CURIE forms. This requires a way to convey the prefix maps from one format to another, so that we can provide those maps to the OBO writer.

@gouttegd
Copy link
Contributor Author

I also want to add that if we go for option D, this should allow to also expand CURIEs in xref tags. The main reason I have not done so in this PR is because cross-references are much more likely to contain non-OBO CURIEs, and so we would be much more likely to run into problems like the one highlighted above.

If we have a way to always let the OBO writer know how to condense arbitrary IRIs into CURIEs, then this would not be a problem anymore.

@matentzn
Copy link

Let's talk a bit about option D before diving into xref, which is furiously discussed here.

  1. Trying to piggy-back on the RDF prefix mechanism which most of the serialiser use. This would involve a the assumption that rdf prefix declaration are the same as prefix maps
  2. Inject a SHACL-based prefix map into the ontology header (can OWLAPI even pass this through?) which is recognised by the OBO parsers (but only the OBO parser). SHACL has a standard vocabulary for prefix declaration, see here for an example
  3. Create a custom RDF extension for idspace maps as ontology annotations, much the same way as all other OBO<-> OWL mappings are implemented. Something like: `http://purl.obolibrary.org/obo/mondo.owl oio:idspace "HGNC: https://identifiers.org/hgnc:". This looks similar to ROBOTs --prefix declation and could be viable if 2 is not doable for OWL API reasons.
  4. Externalise the prefix mapping by requiring a context file as an additional input when interpreting prefix strings in an OBO file. We already have mechanisms to parse these. This may need a new process like "generate context for OBO file()".

I have a slide bias to 3 right now, but I am totally open to arguments in all directions.

@cmungall
Copy link
Member

cmungall commented Aug 23, 2022

I am strongly in favor of @matentzn's option 2. If the shacl namespace vocabulary had existed when we wrote the spec we would have used it.

I tried robot convert -I http://obofoundry.org/registry/obo_prefixes.ttl and it works with all formats (note: for obo serialization it needs to be annotated with an ontology ID)

@gouttegd
Copy link
Contributor Author

Proof-of-concept (mostly to check that I understand option 2 correctly, the code is ugly):

The following OBO file:

format-version: 1.2
ontology: test
idspace: myprefix http://example.org/myp

[Term]
id: test:001
name: test term
replaced_by: myprefix:002

gives the following OFN file, where the idspace tag has been used to (a) expand the myprefix:002 prefix into http://example.org/myp002 and (b) add SHACL annotations:

Ontology(<http://purl.obolibrary.org/obo/test.owl>
Annotation(<http://www.geneontology.org/formats/oboInOwl#hasOBOFormatVersion> "1.2")
Annotation(<http://www.w3.org/ns/shacl#declare> _:genid2147483648)

Declaration(Class(<http://purl.obolibrary.org/obo/test_001>))
Declaration(AnnotationProperty(<http://purl.obolibrary.org/obo/IAO_0100001>))
Declaration(AnnotationProperty(<http://www.geneontology.org/formats/oboInOwl#hasOBOFormatVersion>))
Declaration(AnnotationProperty(<http://www.geneontology.org/formats/oboInOwl#id>))
Declaration(AnnotationProperty(<http://www.w3.org/2000/01/rdf-schema#label>))
Declaration(AnnotationProperty(<http://www.w3.org/ns/shacl#declare>))
Declaration(AnnotationProperty(<http://www.w3.org/ns/shacl#namespace>))
Declaration(AnnotationProperty(<http://www.w3.org/ns/shacl#prefix>))
############################
#   Annotation Properties
############################

# Annotation Property: <http://purl.obolibrary.org/obo/IAO_0100001> (term replaced by)

AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#label> <http://purl.obolibrary.org/obo/IAO_0100001> "term replaced by")

# Annotation Property: <http://www.geneontology.org/formats/oboInOwl#hasOBOFormatVersion> (has_obo_format_version)

AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#label> <http://www.geneontology.org/formats/oboInOwl#hasOBOFormatVersion> "has_obo_format_version")



############################
#   Classes
############################

# Class: <http://purl.obolibrary.org/obo/test_001> (test term)

AnnotationAssertion(<http://purl.obolibrary.org/obo/IAO_0100001> <http://purl.obolibrary.org/obo/test_001> <http://example.org/myp002>)
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#id> <http://purl.obolibrary.org/obo/test_001> "test:001")
AnnotationAssertion(<http://www.w3.org/2000/01/rdf-schema#label> <http://purl.obolibrary.org/obo/test_001> "test term")


AnnotationAssertion(<http://www.w3.org/ns/shacl#namespace> _:genid2147483648 "http://example.org/myp")
AnnotationAssertion(<http://www.w3.org/ns/shacl#prefix> _:genid2147483648 "myprefix")
)

When converting back to OBO, the SHACL annotations are used to (a) condense back the expanded IRI into its original CURIE form, and (b) restore the original idspace tag:

format-version: 1.2
idspace: myprefix http://example.org/myp 
ontology: test

[Term]
id: test:001
name: test term
replaced_by: myprefix:002

Is that what you both had in mind?

@cmungall
Copy link
Member

perfecto!!!!

@gouttegd
Copy link
Contributor Author

Great. In that case I don’t see any obstacle against option 2.

The first part (idspace to SHACL annotations, when parsing a OBO file) is trivial to do.

The second part (SHACL to idspace tags and IRIs to CURIEs, when writing a OBO file) requires more work to be done properly (for now I had to do some violence to the Owl2Obo code to get it to work), but is definitely doable.

Of course, if it can be done with SHACL annotations, then logically it could also be done with any other annotation intended to store prefix mappings, so option 3 remains a possibility. But I don’t see the point of creating another annotation, when we already have the SHACL ones.

@matentzn
Copy link

Out of curiosity, how does:

AnnotationAssertion(<http://www.w3.org/ns/shacl#namespace> _:genid2147483648 "http://example.org/myp")
AnnotationAssertion(<http://www.w3.org/ns/shacl#prefix> _:genid2147483648 "myprefix")

work with owlapi? what is the type of _:genid2147483648?

@gouttegd
Copy link
Contributor Author

It’s a OWLAnonymousIndividual. The ID is automatically generated by OWLDataFactory.getAnonymousIndividual().

It would be possible to use an explicit IRI instead (the SHACL specification explicitly allows to use either a IRI or a blank node), but since its only purpose is to “link” the prefix annotation with the namespace annotation, I don’t think there would be anything to gain in doing that?

@matentzn
Copy link

Would be interesting to experiment how this would affect module extraction for example. Most likely it would not - since they are ontology annotations, you will have to copy them forward if you want to extract -> convert. Also, a default behaviour for namespace clashes is necessary when multiple ontologies with prefix maps are merged.

@gouttegd
Copy link
Contributor Author

About conflicting prefix maps: ontologies are merged after they have been parsed, so by the time two (or more) OBO-format ontologies with conflicting maps are merged, the CURIEs they contain will have already been expanded into IRIs — so the resulting merged ontologie will at least contain correct IRIs.

The problem will be when the merged ontology is saved in OBO format. Then:

  • if robot merge has been called with --include-annotations false (the default): Only the shacl#declare annotation from the first input ontology is carried over to the merged ontology, and IRIs will be condensed back to CURIEs using only the prefix map from that first ontology. This may or may not be what the user wanted or expected, but there is no risk of conflicts.
  • with --include-annotations true: shacl#declare annotations from all the input ontologies are carried over to the merged ontology, so conflicts can happen.
    • In case of conflicting namespaces (same prefix for different namespaces, e.g., ontology A has mapped ex to http://example.org/foo while ontology B has mapped ex to http://example.org/bar), completely different IRIs will be condensed to identical CURIEs (e.g., http://example.org/foo001 and http://example.org/bar001 will both be condensed to ex:001).
    • In case of conflicting prefixes (different prefixes for same namespace, e.g., ontology A has mapped ex to http://example.org/foo while ontology B has mapped another prefix fo to that same namespace), IRIs in the affected namespace will be condensed to CURIEs using only of the prefixes — the way it is currently implemented, the last corresponding prefix found in the map.

The case of conflicting namespaces is obviously the most problematic one, since it will lead to incorrect CURIEs. Not sure what the best course of action would be to deal with that case:

  • detect the conflict in OWL API and emit a warning?
  • detect the conflict in OWL API and silently altering one of the prefixes (e.g., if http://example.org/foo and http://example.org/bar are both mapped to ex, remap the second to ex1)?
  • don’t do anything in the OWL API itself (let the conflict happen, along with its consequences), but encourage users to have a dedicated QC check that looks for conflicting prefix maps in SHACL annotations (a test that could be added to the ODK)?
  • encourage users to forcibly strip SHACL prefix maps away when merging ontologies and to inject their own SHACL annotations instead, so they have full control over which prefixes are used?

@gouttegd
Copy link
Contributor Author

Independently of the risk of conflicts, I note that something interesting is happening when ontologies are merged with --include-annotations false.

Let’s say we have the following SHACL annotations in ontology A:

Annotation(sh:declare _:genid2147483648)
AnnotationAssertion(sh:namespace _:genid2147483648 "http://example.org/foo")
AnnotationAssertion(sh:prefix _:genid2147483648 "fo")

And the following annotations in ontology B:


Annotation(sh:declare _:genid2147483648)
AnnotationAssertion(sh:namespace _:genid2147483648 "http://example.org/bar")
AnnotationAssertion(sh:prefix _:genid2147483648 "ba")

The merged ontology will contain:


Annotation(sh:declare _:genid2147483648)
AnnotationAssertion(sh:namespace _:genid2147483648 "http://example.org/foo")
AnnotationAssertion(sh:prefix _:genid2147483648 "fo")
AnnotationAssertion(sh:namespace _:genid2147483649 "http://example.org/bar")
AnnotationAssertion(sh:prefix _:genid2147483649 "ba")

Notice how only the ontology annotation from ontology A (the one linking to the fo prefix mapping) is present. The ontology annotation from B (linking to the ba prefix mapping) is absent, as expected with --include-annotations false. However, the annotations on the anonymous individual from ontology B, are present.

Because they are not “linked” to an ontology annotation, they would not be found when we are looking for SHACL prefix maps, so they cannot cause a conflict (so the meaning of the --include-annotations false option is preserved). But I wonder if we should try to get rid of such “floating” annotations?

@cmungall
Copy link
Member

@gouttegd - are you sure that is the behavior on merging with blank nodes? That would indicate a problem with the owlapi AFAIK

I think it's good to think through behavior with these annotations. We need to define behavior in the case of conflicting prefixes. I suggest

  • in strict mode an error is thrown if the shacl prefixmap is not bijective
  • in non-strict mode, if one prefix has multiple namespaces an arbitrary one is chosen
  • in non-strict mode, if one namespace has multiple prefixes then contraction will choose an arbitrary one

However it should be left as a problem for the user to make sure the shacl namespaces are well behaved. Merging is one way to introduce problem states, but I don't think this should be treated as an obo format issue. We can have some lightweight checks and operations at the robot level.

@gouttegd
Copy link
Contributor Author

gouttegd commented Aug 25, 2022

@cmungall

are you sure that is the behavior on merging with blank nodes? That would indicate a problem with the owlapi AFAIK

This is what I observe when merging the following file:

test1.ofn
Prefix(oboInOwl:=<http://www.geneontology.org/formats/oboInOwl#>)
Prefix(sh:=<http://www.w3.org/ns/shacl#>)
Prefix(rdfs:=<http://www.w3.org/2000/01/rdf-schema#>)

Ontology(<http://purl.obolibrary.org/obo/test1.owl>
Annotation(oboInOwl:hasOBOFormatVersion "1.2")
Annotation(sh:declare _:genid2147483648)

Declaration(Class(<http://purl.obolibrary.org/obo/test_001>))
Declaration(AnnotationProperty(oboInOwl:hasOBOFormatVersion))
Declaration(AnnotationProperty(oboInOwl:id))
Declaration(AnnotationProperty(rdfs:label))
Declaration(AnnotationProperty(sh:declare))
Declaration(AnnotationProperty(sh:namespace))
Declaration(AnnotationProperty(sh:prefix))

AnnotationAssertion(rdfs:label oboInOwl:hasOBOFormatVersion "has_obo_format_version")

AnnotationAssertion(oboInOwl:id <http://purl.obolibrary.org/obo/test_001> "test:001")
AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/test_001> "test term")

AnnotationAssertion(sh:namespace _:genid2147483648 "http://example.org/foo")
AnnotationAssertion(sh:prefix _:genid2147483648 "fo")
)

with this one (almost identical, but different prefix map):

test2.ofn
Prefix(oboInOwl:=<http://www.geneontology.org/formats/oboInOwl#>)
Prefix(sh:=<http://www.w3.org/ns/shacl#>)
Prefix(rdfs:=<http://www.w3.org/2000/01/rdf-schema#>)

Ontology(<http://purl.obolibrary.org/obo/test2.owl>
Annotation(oboInOwl:hasOBOFormatVersion "1.2")
Annotation(sh:declare _:genid2147483648)

Declaration(Class(<http://purl.obolibrary.org/obo/test_002>))
Declaration(AnnotationProperty(oboInOwl:hasOBOFormatVersion))
Declaration(AnnotationProperty(oboInOwl:id))
Declaration(AnnotationProperty(rdfs:label))
Declaration(AnnotationProperty(sh:declare))
Declaration(AnnotationProperty(sh:namespace))
Declaration(AnnotationProperty(sh:prefix))

AnnotationAssertion(rdfs:label oboInOwl:hasOBOFormatVersion "has_obo_format_version")

AnnotationAssertion(oboInOwl:id <http://purl.obolibrary.org/obo/test_002> "test:002")
AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/test_002> "another test term")

AnnotationAssertion(sh:namespace _:genid2147483648 "http://example.org/bar")
AnnotationAssertion(sh:prefix _:genid2147483648 "ba")
)

with robot convert -i test1.ofn -i test2.ofn -o merged.ofn

merged.ofn
Prefix(:=<http://purl.obolibrary.org/obo/test1.owl#>)
Prefix(sh:=<http://www.w3.org/ns/shacl#>)
Prefix(owl:=<http://www.w3.org/2002/07/owl#>)
Prefix(rdf:=<http://www.w3.org/1999/02/22-rdf-syntax-ns#>)
Prefix(xml:=<http://www.w3.org/XML/1998/namespace>)
Prefix(xsd:=<http://www.w3.org/2001/XMLSchema#>)
Prefix(rdfs:=<http://www.w3.org/2000/01/rdf-schema#>)
Prefix(oboInOwl:=<http://www.geneontology.org/formats/oboInOwl#>)


Ontology(<http://purl.obolibrary.org/obo/test1.owl>
Annotation(oboInOwl:hasOBOFormatVersion "1.2")
Annotation(sh:declare _:genid2147483648)

Declaration(Class(<http://purl.obolibrary.org/obo/test_001>))
Declaration(Class(<http://purl.obolibrary.org/obo/test_002>))
Declaration(AnnotationProperty(oboInOwl:hasOBOFormatVersion))
Declaration(AnnotationProperty(oboInOwl:id))
Declaration(AnnotationProperty(rdfs:label))
Declaration(AnnotationProperty(sh:declare))
Declaration(AnnotationProperty(sh:namespace))
Declaration(AnnotationProperty(sh:prefix))

AnnotationAssertion(rdfs:label oboInOwl:hasOBOFormatVersion "has_obo_format_version")

AnnotationAssertion(oboInOwl:id <http://purl.obolibrary.org/obo/test_001> "test:001")
AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/test_001> "test term")

AnnotationAssertion(oboInOwl:id <http://purl.obolibrary.org/obo/test_002> "test:002")
AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/test_002> "another test term")


AnnotationAssertion(sh:namespace _:genid2147483648 "http://example.org/foo")
AnnotationAssertion(sh:prefix _:genid2147483648 "fo")
AnnotationAssertion(sh:namespace _:genid2147483649 "http://example.org/bar")
AnnotationAssertion(sh:prefix _:genid2147483649 "ba")
)

Tested with ROBOT 1.8.4 and 1.9.0 (identical results).

@cmungall
Copy link
Member

cmungall commented Sep 2, 2022

@gouttegd - apologies, I didn't see the two bnodes differed by one digit, I thought that the two bnodes from the different graphs were merged into one, which would have been a mistake, had it actually happened.

I don't see any major problems here

one thing that might be annoying is that if I merge N ontologies that all have:

[ sh:prefix "ex"; sh:namespace "https://example.org/"]

then I will get

[ sh:prefix "ex"; sh:namespace "https://example.org/"]
[ sh:prefix "ex"; sh:namespace "https://example.org/"]
...
[ sh:prefix "ex"; sh:namespace "https://example.org/"]

Things would get silly if these were included in extracts as we would end up with exponential growth of parasitic duplicative declarations but I don't think these would be included with extract by default

this will be fixed by a laundry cycle through an id-space aware obo serializer. We can imagine having some lightweight robot commands for removing redundant bnodes, detecting bijectivity conflicts etc

@ignazio1977
Copy link
Contributor

Very interesting and long discussion.

Re merging of blank nodes, parsing two ontologies where the blank nodes have the same ids doesn't cause the in memory blank nodes to clash - blank node ids are remapped during parsing. The node on file will match the triples of the node in memory but the id will be recomputed. This is precisely to avoid clashes (think for example during import resolution; same concept during merges).

The behaviour is optional and can be disabled - removing remapping can be useful in some situations, but it brings with it the risk of clashes and is not compliant with the specs, so by default remapping is on.

@ignazio1977
Copy link
Contributor

@gouttegd @cmungall I'm trying to figure out if you believe there are remaining issues or if the pull request is complete and can be merged

@gouttegd
Copy link
Contributor Author

gouttegd commented Oct 3, 2022

@ignazio1977 It depends on how we answer @balhoff ’s question above: “How much do we want to try to handle roundtrip with these changes?”

For now, we do not try to “handle roundtrip”. As a result, if an ontology in OBO format has a prefix declaration in its header:

idspace: HGNC http://www.genenames.org/cgi-bin/gene_symbol_report?hgnc_id=

and uses IDs with this declared HGNC prefix:

[Term]
id: HGNC:100

then this is handled correctly when parsing from OBO to OWL, but not in the other way around, because for now the OBO writer doesn’t know how to “CURIfy” the expanded IRI back to its original prefixed form.

We already agreed that the solution to that is to save the prefix mappings in the ontology as SHACL declarations, and I have some code to do that in another branch, but it’s not ready to merge yet.

@balhoff
Copy link
Contributor

balhoff commented Dec 9, 2022

@gouttegd what's the status of your SHACL prefix mappings code? Want to merge it into here?

@gouttegd
Copy link
Contributor Author

gouttegd commented Dec 9, 2022

@balhoff Sorry, I haven’t found any time to work on it since my last message. My code is still nowhere near ready to be merged.

@balhoff
Copy link
Contributor

balhoff commented May 11, 2023

FYI, I've rebased this PR onto the version4 branch and am adding roundtrip support by making the OBODocumentFormat implement PrefixDocumentFormat. I'll hopefully open a new PR soon.

@balhoff
Copy link
Contributor

balhoff commented May 12, 2023

This PR is superseded by #1102.

@balhoff balhoff removed their assignment May 12, 2023
@gouttegd gouttegd closed this May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants