-
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
Robot silently loses axioms for some ontologies #98
Comments
Note OWLTools (usig 4.2.6 of owlapi) has the expected behavior - ie OP is preserved
|
It's probably picking up the ROBOT version, which is 0.0.1-SNAPSHOT. The bug is weird. |
I've never seen this before. It started when I upped version of the owlapi in #95 I have used this version of the owlapi elsewhere with no issues (this is what is used in the owltools export above, it signs it However, perhaps somehow the act of upping the version changed things unexpectedly. I notice robot has exclusions for elk in the pom, owltools does not clutching at straws... |
If you run |
Thanks @balhoff! Sorry, but I can't help with the debugging this week. |
I have a guess that it is some XML weirdness from some classes overwriting one another when the combined jar is made (if that makes sense...). I tried adding the one-jar plugin, which uses some custom class-loading magic, to the plugins section of <plugin>
<groupId>com.jolira</groupId>
<artifactId>onejar-maven-plugin</artifactId>
<version>1.4.4</version>
<executions>
<execution>
<configuration>
<mainClass>org.obolibrary.robot.CommandLineInterface</mainClass>
<onejarVersion>0.97</onejarVersion>
<attachToBuild>true</attachToBuild>
</configuration>
<goals>
<goal>one-jar</goal>
</goals>
</execution>
</executions>
</plugin> If you run
|
Thanks! I will experiment more with the pom when I get a chance. Another experiment would be to try making it more like the owltools pom (or conversely, making the owltools one more like robot until we replicate the issue). Do we know which version of the owlapi the erroneous version is picking up? I have never seen the behavior in the various iterations of the owlapi, if it's picking up something on the master branch of owlapi we should report this. |
If you revert to OWL API 4.2.4, the same pattern happens (failure in flattened jar, works in one-jar). But it is not a silent failure:
If you make an entirely new pom.xml with only one dependency, |
Okay I think I've got it. Inside the various jars are some metadata files within top-level
If you edit the jar and append one more line to
Then run, the program will execute correctly. I think the answer may be to just switch to building the jar with the one-jar plugin, which avoids squashing all the jars together (it archives them separately inside the resulting jar, and then sets up a special class loader to access them correctly). This would also correct the problem of the OWL API version being printed correctly. |
Nice. I'm fine with one-jar if that's the fix.
|
Not 100% sure how to do this - is this an easy PR? |
Oh yeah I was meaning to get back to this. I'll submit a PR.
|
Is there any way to avoid all the |
- add regression test for #98 - revert POM to pre-OneJar, then replace maven-assembly-plugin with maven-shade-plugin
I spent most of my day trying to get rid of the OneJar warnings, and came up with #106. OneJar is ancient and unmaintained. Despite various advice on the web, I couldn't suppress its 40 lines of warnings. I dug in and tried to resolve the dependency conflict. It's almost certainly caused by a conflict between with The immediate fix is to change Using
This seems to work: No more OneJar, and no more warning lines. Maven Shade is maintained, at least, and I think they're accomplishing the same thing. Now that I see all the gory details of this problem and solution, worried about side-effects. This is Shades' build-time warning:
Any opinions? |
Ok, I merged #106 which includes a regression test for this problem, so I'm closing this. |
@jamesaoverton thanks for cleaning this up! |
I have to reopen this one for a quick discussion, as the new OWLAPI does not seem to permit this IRI, see commit: In one of our integration tests. The Test simply fails, and Protege does not open it. Note this is not a big problem as this issue here is still "fixed" - in that it fails hard (not failed silently). Maybe instead of dropping the bad axiom we should make the test expect the failure? Q to @jamesaoverton: Why did this not show up in our migration to 4.5.24? Because I tried opening the file in Protege (which is using 4.5.24) and it definitely does not open! |
I can reproduce this: regression test passes in 4.5.24 and fails in 4.5.25. There were only two commits to OWLAPI, and I can't see how they would affect this: https://github.com/owlcs/owlapi/commits/version4. In ROBOT 1.9.2 (with OWLAPI 4.5.24) the regression test is not actually working. When I run: > java -jar robot-1.9.2.jar convert -i docs/examples/dropped_axiom.owl -o dropped_axiom.owl I can see that the axiom is silently dropped from > java -jar robot-1.9.2.jar diff -l docs/examples/dropped_axiom.owl -r dropped_axiom.owl
Ontologies are identical So the small changes in 4.5.25 are causing a hard fail, which is better than silently dropping axioms. I guess I'm fine with removing the regression test, which wasn't actually working. ROBOT being more strict here may cause more pipeline failures. The error message you get is good, but since it's a parser error the message is buried with all the other attempted parsers. Sigh. |
Thanks for checking. Its a great failure to have.. I will support people fixing this. |
I dropped this test in #1181 because OWLAPI seems to be taking care of it by throwing an error. |
test.owl:
Note the OP is silently lost.
The root cause is presumably the strange URI for the AP. However, if this is invalid an error should be thrown. If it is not invalid then the OP should pass through.
There is a hint in the OWLAPI version. Why does it say
OWL API (version 0.0.1-SNAPSHOT)
. The pom says to get 4.2.6The text was updated successfully, but these errors were encountered: