INCATools / ontology-development-kit

Bootstrap an OBO Library ontology
http://incatools.github.io/ontology-development-kit/
BSD 3-Clause "New" or "Revised" License
212 stars 53 forks source link

Handling missing externally defined synonym type declarations #947

Open matentzn opened 8 months ago

matentzn commented 8 months ago

We have two distinct issues caused by the fact that we are now, for the first time, importing externally declared synonym types into uberon (and other ODK driven ontologies):

  1. Because QC depends on SRCMERGED, externally imported synonym types are not “declared”, i.e. subproperty of synonymtypeproperty.
  2. Because base process does a robot remove --imports all externally declared synonym types lose their declaration in the base file.

The consequence of these is that the resulting OBO file format serialisations become invalid (missing synonym type declarations). I just tried it: Serialising an owl file into OBO without the subproperty axiom simply does not include it.

There are many things to be said here, including that OBO format should not require this obvious syntactic sugar. In the absence of this though, I think we need to inject

query --update ../sparql/inject-subset-declaration.ru --update ../sparql/inject-synonymtype-declaration.ru  \

During both

$(SRCMERGED): $(EDIT_PREPROCESSED) $(OTHER_SRC)
    $(ROBOT) remove --input $< --select imports --trim false \
        merge  $(patsubst %, -i %, $(OTHER_SRC)) \
        query --update ../sparql/inject-subset-declaration.ru --update ../sparql/inject-synonymtype-declaration.ru  -o $@

and

ROBOT_RELEASE_IMPORT_MODE_BASE=$(ROBOT) remove --input $< --select imports --trim false merge $(patsubst %, -i %, $(OTHER_SRC)) query --update ../sparql/inject-subset-declaration.ru --update ../sparql/inject-synonymtype-declaration.ru 

Unless anyone has a better idea.

gouttegd commented 8 months ago

Thank you for highlighting that there are two different issues here. On the Slack discussion I had completely missed the second problem (“synonym-type-declarations-are-missing-from-base-product”) entirely, focusing instead on the first (“synonym-type-declarations-are-missing-during-QC”).

For that second problem, I would agree with your proposed solution:

ROBOT_RELEASE_IMPORT_MODE_BASE=$(ROBOT) remove --input $< --select imports --trim false merge $(patsubst %, -i %, $(OTHER_SRC)) query --update ../sparql/inject-subset-declaration.ru --update ../sparql/inject-synonymtype-declaration.ru 

For the first however, injecting fabricated synonym type definitions as needed (which is basically what inject-synonymtype-declarations.ru is doing) into $(SRCMERGED) would mean that the check for missing synonym type declarations would never catch anything. In the event that we are really missing a synonym type declaration (the synonym type is not declared anywhere, be it in the -edit file or in an import), we might never realise it because the fabricated declaration would always keep the QC check happy. In effect the check for missing synonym type declarations would be absolutely pointless.

What about having two distinct runs of ROBOT report? One on a version of the ontology that excludes imports (e.g. $(SRCMERGED), as is currently the case), and one on a version that includes them. The missing_synonymtype_declaration test would only be enabled (or at least, only enabled with ERROR severity) in the latter.

matentzn commented 8 months ago

What about having two distinct runs of ROBOT report?

This adds a long runtime. Maybe the actually correct way to deal with it is to pretend that

:synonym_type owl:subPropertyOf :SynonymTypeProperty

Is functionally equivalent to:

:synonym_type rdf:type owl:AnnotationProperty

i.e, serves as "subset declaration" - then we could just wiggle out of this here, by saying "they have to be declared".

gouttegd commented 8 months ago

Sorry, but either I don’t understand what you are proposing or I don’t understand how it would solve anything.

What do you mean, “pretend that ...“? Where would we pretend something like that? Are you suggesting a change in how the OWL API treats such axioms?

What do you mean, “we could just say, ‘they have to be declared‘”? Those synonym types are declared – just not in the -edit file.

matentzn commented 8 months ago

What I am saying is this:

Module extraction processes, base generation processes need to know about the fact that these annotations are essential, and either inject them after the fact, or consider them during export.

I think injecting after the fact is fine, as OWL API, for example, injects missing class declarations as well:

If you load/safe this ontology:

SubClassOf(a:a, a:b)

You will end up with

Declaration(a:a)
Declaration(a:b)
SubClassOf(a:a, a:b)

After the serialisation (at least in functional syntax)