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

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

Open
ghost opened this issue Aug 17, 2023 · 10 comments
Open

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

ghost opened this issue Aug 17, 2023 · 10 comments
Labels
1.4 major issues issues that are priority to address in 1.6

Comments

@ghost
Copy link

ghost commented Aug 17, 2023

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?

@ghost ghost added the 1.4 major issues issues that are priority to address in 1.6 label Aug 17, 2023
@anitacaron
Copy link

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
Copy link
Member

balhoff commented Aug 17, 2023

@bvarner-ebi @anitacaron it's possible this is already fixed by owlcs/owlapi#1099

I think that change should be in the next ROBOT release, assuming this gets merged: ontodev/robot#1135

@ghost
Copy link
Author

ghost commented Aug 18, 2023

Thank you for the info, @balhoff. There is no assignee on ontodev/robot#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 obophenotype/uberon#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
Copy link

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
Copy link

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

@ghost
Copy link
Author

ghost commented Aug 18, 2023

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

Thank you, @matentzn and @anitacaron! The checks on obophenotype/uberon#3021 have passed now.

@gouttegd
Copy link

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
Copy link

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
Copy link

gouttegd commented Aug 19, 2023

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
Copy link

@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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.4 major issues issues that are priority to address in 1.6
Projects
None yet
Development

No branches or pull requests

5 participants