Rewrote phyloreference testing code #6
Conversation
| * | ||
| */ | ||
| public class JPhyloRef | ||
| { | ||
| /** Version of JPhyloRef */ | ||
| public static final String VERSION = "0.0.1-SNAPSHOT"; |
There was a problem hiding this comment.
I want to keep the version number in sync with the version in the Maven project model (pom.xml), and those should include "SNAPSHOT" if they're still in development. We might release something eventually, but it's also possible that JPhyloref might remain permanently in development, since most of the stuff we use it for now could be outsourced to better tools like Owlery. Does that make sense? Otherwise, I'm happy to use semantic versioning here and to add '-SNAPSHOT' to the version just in the pom.xml file.
There was a problem hiding this comment.
I'm not necessarily opposed to the SNAPSHOT part. But surely we're past v0.0.1 with this, or are you suggesting we're not?
There was a problem hiding this comment.
You're right! I've bumped the version number to 0.1 and added a Changelog so we can keep track of version changes (97e0676). I'll tag this commit as v0.1 once I've merged it into master.
| public void execute(String[] args) { | ||
| // Display version information. | ||
| System.err.println("jphyloref/" + VERSION + "\n"); |
There was a problem hiding this comment.
It seems bad practice to print something to STDERR without control by the user, and not signifying any error or potential error condition.
| for(Command cmd: commands) { | ||
| if(cmd.getName().equalsIgnoreCase(command)) { | ||
| // match! | ||
| // Found a match! | ||
| cmd.execute(cmdLine); | ||
| System.exit(0); | ||
| return; |
There was a problem hiding this comment.
What's the point of the return; statement?
| cmd.execute(cmdLine); | ||
| System.exit(0); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Could not find any command. | ||
| System.err.println("Error: command '" + command + "' has not been implemented."); | ||
| System.exit(1); | ||
| return; |
There was a problem hiding this comment.
What's the point of the return statement?
| public void execute(CommandLine cmdLine) { | ||
| // No arguments are provided. | ||
|
|
||
| // Display a synopsis. | ||
| System.err.println("Synopsis: jphyloref <command> <options>\n"); |
There was a problem hiding this comment.
Help and synopsis should be printed to STDOUT, not STDERR.
| System.err.println("Where command is one of:"); | ||
| for(Command cmd: commands) { | ||
| // Display the description of the command. | ||
| System.err.println(" - " + cmd.getName() + ": " + cmd.getDescription()); |
There was a problem hiding this comment.
See above for STDERR versus STDOUT. This is help messages, right?
| * | ||
| * @author Gaurav Vaidya <[email protected]> | ||
| * | ||
| */ | ||
| public class TestCommand implements Command { | ||
| /** | ||
| * This command is named "test". It should be | ||
| * This command is named Test. It should be | ||
| * involved "java -jar jphyloref.jar test ..." |
| @@ -15,32 +15,86 @@ | |||
| import org.semanticweb.owlapi.vocab.OWLRDFVocabulary; | |||
|
|
|||
| /** | |||
| * OWLHelper contains helpful functions. | |||
| * OWLHelper contains methods that make it that make it easy to access information encoded in OWL. | |||
| public static Set<OWLNamedIndividual> getPhyloreferences(OWLOntology ontology, OWLReasoner reasoner) { | ||
| // Get a list of all phyloreferences. First, we need to know what a Phyloreference is. | ||
| Set<OWLEntity> set_phyloref_Phyloreference = ontology.getEntitiesInSignature(IRI_PHYLOREFERENCE); | ||
| if (set_phyloref_Phyloreference.isEmpty()) { | ||
| throw new RuntimeException("Class 'phyloref:Phyloreference' is not defined in ontology."); | ||
| throw new RuntimeException("Class " + IRI_PHYLOREFERENCE + " is not defined in ontology."); | ||
| } |
There was a problem hiding this comment.
Is this a programmer or internal error, or is it an illegal argument case? If the latter, use the appropriate exception.
| } | ||
| if (set_phyloref_Phyloreference.size() > 1) { | ||
| throw new RuntimeException("Class 'phyloref:Phyloreference' is defined multiple times in ontology."); | ||
| throw new RuntimeException("Class " + IRI_PHYLOREFERENCE + " is defined multiple times in ontology."); |
… STDERR, replaced RuntimeException with IllegalArgumentException and removed unnecessary version output as per suggestions from @hlapp.
|
@hlapp Thanks for the comments! I've fixed everything you suggested except the version number. Let me know if you still think I should change that; otherwise, I'll go ahead and merge this! |
This pull request adds the publication [Brochu 2003](http://dx.doi.org/10.1146/annurev.earth.31.100901.141308) as curated by the Curation Tool. This required updated testcase2owl.py to retain information on the JSON-LD `@context` and `pso:holdsStatusInTime`, adds some definitions from the TimeInterval ontology, which we can't import directly as it results in an inconsistent ontology, fixed an incorrect error message, removed `@id` as a necessary field (closes #15), and add a `labels` term to PHYX file to represent sets of labels. It also incorporates JPhyloref improvements (phyloref/jphyloref#6, phyloref/jphyloref#9) that implements statuses for phyloreferences (phyloref/klados#25).
The current version of the phyloreference testing code had evolved through several different cycles, was badly written, and appeared to not be correctly testing phyloreferences using the
expected_phyloreference_namedproperty. I replaced it with cleaner, easier to read code that appears to be testing phyloreferences correctly.This has now been tested by its incorporation into #9 and it's use in phyloref/clade-ontology#29, so it's ready to be reviewed and merged.