-
Notifications
You must be signed in to change notification settings - Fork 170
Description
Hi!
this is just to record a small inconsistency in the behavior from 2.6 to 2.7. In particular, while the javadoc for InChIGeneratorFactory.getInChIGenerator(IAtomContainer container, String options) does not specify how to separate the options in the string, the underlying InChIGenerator(IAtomContainer atomContainer, String optStr, boolean ignoreAromaticBonds) specifies "optStr: Space delimited string of options...".
From release 2.6 to 2.7, the behavior has changed from accepting only space separated to accepting only comma separated string. In 2.7 space-separated options trigger the "Unrecognised InChI option" warning message.
Here is how to see the different behavior:
SmilesParser sp = new SmilesParser(DefaultChemObjectBuilder.getInstance());
IAtomContainer iac = sp.parseSmiles("C[C@H](Cl)F");
InChIGeneratorFactory factory = InChIGeneratorFactory.getInstance();
try {
InChIGenerator genComma = factory.getInChIGenerator(iac,"FixedH, SNon");
System.out.println("With comma-separated options: "+genComma.getInchi());
} catch (CDKException e) {
System.out.println("With comma-separated options: "+e.getMessage());
}
InChIGenerator genSpace = factory.getInChIGenerator(iac,"FixedH SNon");
System.out.println("With space-separated options: "+genSpace.getInchi());
With release 2.6 (from maven) the output is:
With comma-separated options: InChI generation failed: Unrecognised InChI option
With space-separated options: InChI=1/C2H4ClF/c1-2(3)4/h2H,1H3
With release 2.7.1 (from maven) the output is:
With comma-separated options: InChI=1/C2H4ClF/c1-2(3)4/h2H,1H3
org.openscience.cdk.inchi.InChIOptionParser WARN: Ignore unrecognized InChI flag:null
With space-separated options: InChI=1S/C2H4ClF/c1-2(3)4/h2H,1H3/t2-/m1/s1
Using getInChIGenerator(IAtomContainer container, InchiOptions options) is a way to by-pass the parsing problem entirely.
Still, I wanted to fix to the documentation adding which separator is expected, but then I realized I do not know if the change of separator from space to comma is intentional. So, is it intentional?