-
Notifications
You must be signed in to change notification settings - Fork 79
Add --input-format option
#1269
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
Add --input-format option
#1269
Conversation
Add a common `--input-format` option which accepts the same values as the `--output-format` option, but indicates the expected format of the input file (as loaded by the `CommandLineHelper.updateInputOntology` method).
Update the `CommandLineHelper.getInputOntologies()` methods so that they too use the `--input-format` option if it is present in the command line.
Mention the `--input-format` option in the global documentation. Add a basic test in which we attempt to load a OFN-formatted file that imports a OWL file, first with the correct format, then with an incorrect format.
matentzn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great to me, I left a few comments for the next reviewer (not for you @gouttegd). The diff is a bit suboptimal for an easy review, but i got it in the end
- You did not remove any method ✅
- Your changes are documented ✅
- The default behaviour appears to be preserved (based my reading of FileDocumentSource(x,y)) ✅
@balhoff can you also to a sweep?
| // Otherwise load from file using default method | ||
| return loadOntology(manager, new FileDocumentSource(ontologyFile)); | ||
| OWLDocumentFormat fmt = inputFormat != null ? getFormat(inputFormat) : null; | ||
| return loadOntology(manager, new FileDocumentSource(ontologyFile, fmt)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to next reviewer: I expect that FileDocumentSource(,) handles the case where fmt is null by trying to "guess" the format as usual. This is the most important assumption in this PR as it basically ensures that the old functionality prevails when no --input-format is supplied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Calling the FileDocumentSource constructor with a null argument for the document format is basically the same thing as calling the 1-argument constructor. In fact the 1-argument constructor does nothing else but calling another constructor with a null argument for the format.
balhoff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gouttegd! I approve, but check my one code comment.
|
Much appreciated! |
Resolves [#1038]
docs/have been added/updatedmvn verifysays all tests passmvn sitesays all JavaDocs correctCHANGELOG.mdhas been updatedThis PR is another attempt to implement the
--input-formatoption requested in #1038. The difference with #1056 is that instead of restricting the parsers available to the OWLManager (which has the side effect that imports cannot be parsed if they happen to be in a different format than the main ontology), here we simply explicitly pass aOWLDocumentFormatobject when constructing theOWLOntologyDocumentSource(as suggested by @balhoff here).