-
Notifications
You must be signed in to change notification settings - Fork 74
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
Improvements to report
command
#391
Comments
Good point, this is underspecified at the moment. There was an analogous issue with the Should we have equivalent options for report? However, the defaults may have to be different, because as you point out any ontology that has imports and references classes in the import will produce false positive errors. @jamesaoverton @rctauber how do you feel about having the defaults be different here? |
report
command seems to merge imports during queriesreport
command does not merge imports during queries, resulting in false positives
I would think that you’re only reporting on your base ontology. That way,
issues with other ontologies won’t break the report.
That said, it might make sense to have an option for including imports
anyway, which would make it easier to report on the edit versions of
ontologies who use templates like OBI.
…On Tue, Oct 30, 2018 at 18:10 Chris Mungall ***@***.***> wrote:
Good point, this is underspecified at the moment.
There was an analogous issue with the query command: #158
<#158>
see the fix: #328 <#328> and the
instructions http://robot.obolibrary.org/query
Should we have equivalent options for report?
However, the defaults may have to be different, because as you point out
any ontology that has imports and references classes in the import will
produce false positive errors. @jamesaoverton
<https://github.com/jamesaoverton> @rctauber <https://github.com/rctauber>
how do you feel about having the defaults be different here?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#391 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AP-WRjGb7YXzrg-cZBixy5PDxp9Xi4n_ks5uqM5RgaJpZM4YC9L6>
.
|
I think what is happening here is that there is leakage of declarations from the import into the base. E.g. if base has an axiom What will then happen is that it will report that Y has no label. I'm not sure of a good way around this.
Yes, this is a problem with my proposal to bring in the import Not sure of the best way around this |
@cmungall where do you think the I did a small work-around where I also filtered like: |
I believe it's OWLAPI, but can you try some SPARQL to check? |
A declaration with no other axioms is probably an error - should this be a
reporting check? It will always come up as missing a label as well, but
with this extra check, it may be easier to debug.
…On Tue, Oct 30, 2018 at 23:46 Chris Mungall ***@***.***> wrote:
I believe it's OWLAPI, but can you try some SPARQL to check?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#391 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AP-WRsGQgn1IhF_aUUeDrjuQWZPLxmbsks5uqR0HgaJpZM4YC9L6>
.
|
I believe the OWLAPI injects the triple, I can't recall if this is a consequence of the source file being obo |
Yes, annoying
convert this to any other serialization injects the declaration triple. But note this isn't completely unique to obo format. If we remove the import (i.e make it a base file) then the declaration is auto-injected. e.g. if you roundtrip this with robot:
you will see the class and OP declaration injected |
Yes, OWLAPI inserts those declarations into serializations. The SPARQL queries for report are already a bit tricky for reasons like this. We could add code to strip those bare declarations. Or we could annotate stuff with isDefinedBy #390 and only query for those. There are other options but I can't think of anything elegant. |
From discussion in ODK meeting - code could be edited to only check axioms - ignoring declarations. |
Re @dosumis - the query could also ignore any subjects that only have an |
@rctauber Yes, let's try that approach. |
Last Friday I was trying to write a query like you described, @rctauber, but I was having trouble getting it to work. If you have a way to help, I'd love a pointer! |
Hi @dougli1sqrd - please look at my PR at #394 (and if you have time, test it 😄) The relevant code is:
@ontodev/robot-team I tested this on a few different inputs, but if anybody else has time to test it, I would really appreciate it. |
Thanks, we'll test it! |
Ah, cool, so this is working in go-ontology go-edit.obo except for
|
This check is only ignoring entities that only have a declaration triple (
Is translated to:
This is an interesting problem because it's unique to ontologies developed in OBO format... @cmungall & @jamesaoverton - do you have any thoughts on this? Potential options I can think of:
|
Yeah something like your option 2 could work! Just so you know though there are also a couple of properties in go-edit:
And would also have to be dealt with somehow. Maybe we could ignore all properties that are subsets of oboInOwl properties? Or something? |
1 would be difficult for people I vote for 2. Either exclude subProps of oboInOwl:SubsetProperty or targets of oboInOwl:inSubset |
Oops I guess I didn't know what I was talking about. My previous comment, |
@cmungall - I can exclude the subprops. Should it just be for this label check? I'm guessing they could also be excluded from missing & multiple definitions? |
It looks like this same kind of error is happening for
This is saying GO:0000004 was deprecated because it was merged. And it's merged into GO:0008151 biological process. This is the owl for GO:0000004:
I guess it's catching this because GO:0000004 is deprecated? |
This is standard for 50% of ontologies
Half make a subclass of ObsoleteClass
The other half (including all obo2owl derived) just have the deprecated
axiom, with no logical axioms
**EDIT** for some reason I thought we were talking about must-have-is-a-parent check. My comment above is irrelevant for labels.
For label checks, if
```
<obo:IAO_0000231 rdf:resource="http://purl.obolibrary.org/obo/IAO_0000227"/>
```
is present (which indicates the class was merged), then do exclude from label checks
|
re subsets: Maybe I'm stating the obvious here - but I don't think we should be checking for labels / definitions of annotation properties. |
@cmungall & @dougli1sqrd - I can include this line in the
That would allow the obsolete classes to use @dosumis - I think that's reasonable. That would solve a lot of the issues with the default annotation properties, as well. What are others' thoughts on this? |
@dosumis Annotation properties should have labels and definitions, like any other term. I don't see why we would exclude them wholesale from these quality checks. @cmungall Is there some reason you can't add labels and definitions to Every one of these "little" exceptions will have to be documented and maintained, and every one of them will make the SPARQL run slower. |
report
command does not merge imports during queries, resulting in false positivesreport
command
unfortunately labels for subsetdefs would get lost in the owl2obo translation. Many ontologies have the edit version maintained in obo for diff/vcs purposes. Subsets have other annoying properties such as hash URIs that don't resolve. |
@rctauber I think your idea would work. I think most/all of those particular false positives are due to merged terms, which will all have that as a |
I think this issue has been resolved. We are working on further improvements to the |
Hello,
I'm attempting to replace the set of robot commands and queries in the geneontology/go-ontology repo that we use to validate changes to the ontology in travis checks and the like with
robot report
. These are run on the go-edit.obo file in that repository. go-edit.obo imports chebi.In
go-ontology/src/ontology/
I runrobot report --input go-edit.obo --output report.tsv --profile profile.txt
, and the profile.txt looks like:The report.tsv indicates about 2000
missing_label
errors on CHEBI terms implying they are in this ontology, but these CHEBI terms are not in the go-edit.obo file, they're imported.So am I doing something wrong in the way I invoke the report command, or on the file that I'm reporting on? Or is this a bug? I see in the code it's supposed to not pull in all the imports I think? Thanks for your help!
The text was updated successfully, but these errors were encountered: