owlcollab / oboformat

Automatically exported from code.google.com/p/oboformat
5 stars 2 forks source link

Breaking issue regarding annotation property 'see also' in Uberon #143

Open ghost opened 1 year ago

ghost commented 1 year ago

Description: Uberon currently has two annotation properties (AP) for the concept "see also":

'see also' http://www.w3.org/2000/01/rdf-schema#seeAlso (>700 uses in Uberon) seeAlso http://www.geneontology.org/formats/oboInOwl#seeAlso (50 uses in Uberon)

Editing an annotation in uberon-edit.obo that initially has AP database_cross_reference by changing the AP to 'see also' and saving generates a third AP:

seeAlso http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso

It then seems impossible to edit and save the AP to be 'see also' http://www.w3.org/2000/01/rdf-schema#seeAlso. This behaviour was confirmed with @anitacaron offline and may be the cause of failing Uberon QC checks.

Can you kindly address so these edits related to using 'see also' can be saved in Uberon?

anitacaron commented 1 year ago

When the uberon-edit.obo is converted to OWL, ROBOT cannot parse it during the usual QC pipeline.

This is the error:

Parser: org.semanticweb.owlapi.rdf.rdfxml.parser.RDFXMLParser@27fde870
    Stack trace:
org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParserException: [line=3185:column=132] IRI 'http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso' cannot be resolved against current base IRI http://purl.obolibrary.org/obo/uberon/materialized.owl reason is: Illegal character in fragment at index 81: http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFXMLParser.parse(RDFXMLParser.java:74)
        uk.ac.manchester.cs.owl.owlapi.OWLOntologyFactoryImpl.loadOWLOntology(OWLOntologyFactoryImpl.java:220)
        uk.ac.manchester.cs.owl.owlapi.OWLOntologyManagerImpl.actualParse(OWLOntologyManagerImpl.java:1303)
        uk.ac.manchester.cs.owl.owlapi.OWLOntologyManagerImpl.loadOntology(OWLOntologyManagerImpl.java:1243)
        uk.ac.manchester.cs.owl.owlapi.OWLOntologyManagerImpl.loadOntologyFromOntologyDocument(OWLOntologyManagerImpl.java:1200)
        org.obolibrary.robot.IOHelper.loadOntology(IOHelper.java:540)
        org.obolibrary.robot.IOHelper.loadOntology(IOHelper.java:425)
        org.obolibrary.robot.IOHelper.loadOntology(IOHelper.java:306)
        org.obolibrary.robot.CommandLineHelper.getInputOntology(CommandLineHelper.java:483)
        org.obolibrary.robot.CommandLineHelper.updateInputOntology(CommandLineHelper.java:581)
[line=3185:column=132] IRI 'http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso' cannot be resolved against current base IRI http://purl.obolibrary.org/obo/uberon/materialized.owl reason is: Illegal character in fragment at index 81: http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.resolveIRI(RDFParser.java:355)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElement.getIDNodeIDAboutResourceIRI(StartRDF.java:340)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElement.startElement(StartRDF.java:307)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElementList.startElement(StartRDF.java:374)
        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.startElement(RDFParser.java:208)
        java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.startElement(AbstractSAXParser.java:510)
        java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractXMLDocumentParser.emptyElement(AbstractXMLDocumentParser.java:183)
        java.xml/com.sun.org.apache.xerces.internal.impl.XMLNSDocumentScannerImpl.scanStartElement(XMLNSDocumentScannerImpl.java:351)
        java.xml/com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl$FragmentContentDriver.next(XMLDocumentFragmentScannerImpl.java:2710)
        java.xml/com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:605)
Illegal character in fragment at index 81: http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso        java.base/java.net.URI.create(URI.java:883)
        java.base/java.net.URI.resolve(URI.java:1066)
        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.resolveFromDelegate(RDFParser.java:277)
        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.resolveIRI(RDFParser.java:346)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElement.getIDNodeIDAboutResourceIRI(StartRDF.java:340)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElement.startElement(StartRDF.java:307)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElementList.startElement(StartRDF.java:374)
        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.startElement(RDFParser.java:208)
        java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.startElement(AbstractSAXParser.java:510)
        java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractXMLDocumentParser.emptyElement(AbstractXMLDocumentParser.java:183)
Illegal character in fragment at index 81: http://www.geneontology.org/formats/oboInOwl#http://www.w3.org/2000/01/rdf-schema#seeAlso        java.base/java.net.URI$Parser.fail(URI.java:2913)
        java.base/java.net.URI$Parser.checkChars(URI.java:3084)
        java.base/java.net.URI$Parser.parse(URI.java:3128)
        java.base/java.net.URI.<init>(URI.java:600)
        java.base/java.net.URI.create(URI.java:881)
        java.base/java.net.URI.resolve(URI.java:1066)
        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.resolveFromDelegate(RDFParser.java:277)
        org.semanticweb.owlapi.rdf.rdfxml.parser.RDFParser.resolveIRI(RDFParser.java:346)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElement.getIDNodeIDAboutResourceIRI(StartRDF.java:340)
        org.semanticweb.owlapi.rdf.rdfxml.parser.NodeElement.startElement(StartRDF.java:307)
balhoff commented 1 year ago

@bvarner-ebi @anitacaron it's possible this is already fixed by https://github.com/owlcs/owlapi/pull/1099

I think that change should be in the next ROBOT release, assuming this gets merged: https://github.com/ontodev/robot/pull/1135

ghost commented 1 year ago

Thank you for the info, @balhoff. There is no assignee on https://github.com/ontodev/robot/pull/1135, but @matentzn appears to be overseeing it. Can you advise when the next ROBOT release is planned?

@anitacaron, in the meantime, to get the changes in https://github.com/obophenotype/uberon/pull/3021 added in the next Uberon release, do you recommend I remove the 'see also' annotations and add them back once this issue is resolved?

matentzn commented 1 year ago

We are planning a ROBOT release for ca second week of September.

If need be, @anitacaron can use odk:dev image to build Uberon, which already has that fix incorporated in ROBOT.

anitacaron commented 1 year ago

I can change the image to odk:dev used in the QC GitHub Action.

ghost commented 1 year ago

I can change the image to odk:dev used in the QC GitHub Action.

Thank you, @matentzn and @anitacaron! The checks on https://github.com/obophenotype/uberon/pull/3021 have passed now.

gouttegd commented 1 year ago

If need be, @anitacaron can use odk:dev image to build Uberon, which already has that fix incorporated in ROBOT.

Did you seriously recommend to switch Uberon to a development snapshot just so that we could get two annotations to pass immediately?

I spend three weeks carefully editing the Makefile line by line, rebuilding everything between two edits to be absolutely sure everything works exactly as it did before, and in the meantime you decide to do things in YOLO mode, what could happen after all?

matentzn commented 1 year ago

I did "decide" nothing, but told @anitacaron she could switch the QC action to dev if (and only if) she has an issue open to revert it on the 7th of September. The only difference here is the version of ROBOT used.

In any case, you are of course right in your rebuke here. I am unfortunately not perfect and suffer from lapses of judgement about 10 times / day, unfortunately this is one of them. @anitacaron what alternative solution to using the dev image do we have? Drop the seeAlso annotations? Temporily use source instead of seeAlso and make an issue to catch up with that after the new ROBOT release?

gouttegd commented 1 year ago

The problem is not so much that you had a lapse of judgment, that the fact that nobody else paused for a minute to consider whether it was really a good idea.

For the record, I think it’s a terribly bad idea, and I would have expected people like @anitacaron or @shawntanzk (who approved the PR after Anita’s edit) to realise it even if the idea came from you.

The proper solution would have been to tell @bvarner-ebi to either remove his annotations from the time being or to hold the PR until the next ODK release. The PR didn’t fix anything critical as far as I can tell – certainly nothing critical enough to warrant switching the entire ontology to an unstable development snapshot!

Now the “issue open to revert on the 7th of September” has been created. As I said there, either we drop the annotations and revert immediately to the latest stable ODK, or we are effectively stuck with the development version until the next stable.

I am personally strongly in favour of the former, if only to avoid creating a precedent and drive the point to every single maintainer, and I can’t believe I have to write this, that you do not switch to a development snapshot just to allow a PR to pass the test suite! Especially not the way it was done here, with the switch happening sneakily in the PR itself instead of being the subject of a dedicated PR!

If you do opt for remaining on dev until the next stable release, I hope you can at least tell me that you’re not going to publish any new dev image in the meantime – that is, you’ll consider dev effectively frozen until the next release.

shawntanzk commented 12 months ago

@gouttegd soz my bad, I didnt think too much about that - probably should have clarified why the switch to dev, will be more careful with it in the future :)