@Ignored some test cases

[uli-f]

I added @Ignore annotation to some of the test cases that fail in PR #15.

@ntk73 Once PR #15 is merged, I would appreciate it if you could have a look. Seems to have something to do with stereochemistry.

[ntk73]

OK, I will check these

[uli-f]

These test cases are now in PR #18.

[ntk73]

@uli-f , I suppose that a possible reason for failures is that RAuxInfo with the coordinates is not given. The stereo information is treated fully only on level 2D or 3D. From the original RInChI examples there are some double conversion tests with stereo info which do not fail, but they have RAuxInfo (e.g. testExample_Inverted_stereochemistry()).

BTW: I do not like this at all, but this is the approach implemented in native code - stereo handling is linked with MDL 2D/3D coordinates.

[uli-f]

Okay, thank you for clarifying.

If there is a clearly defined reason why a test case fails, I see two ways to resolve this:

• Can the missing RAxuInfo data somehow be amended to make sure that we identified the correct reason the test case is failing (e.g., with the help of molecular editor by opening and saving the file and then converting to RInChI)? If that is possible with a reasonable amount of effort I'd prefer that so that we can be sure we figured out why it fails.
• Alternatively, we leave the test case with its @Ignore annotation in the code and give a brief explanation why the test is ignored within the @Ignore annotation: @Ignore("test is ignored as it fails due to ....").

Any thoughts on your end?

[ntk73]

In order to get these tests running, somehow the RAuxInfo should be generated. Some manual work would be needed, perhaps. For example: (1) draw the molecules and generate the 2D coordinates; (2) Store the molecules as MDL files; (3) Make a RXN file with 2D coordinates for the reaction components; (4) Convert the RXN to a couple: (RInChI, RAuxInfo) and then finally and hopefully the tests should run.

[uli-f]

Thank you for listing the necessary steps.

I'll have a look on Friday and see if I can do this for one or two of them. If this fixes the issue we might just drop the other few test cases.

[ntk73]

Few additional tips that might be useful in a practical aspect (I applied these for preparing some of the extra test units)

1. I have used OpenBabel to generate MDL MOL file with 2D coordinates from a SMILES string with stereo information;
2. The separate MDL MOL file text fragments were manually combined to make a single RXN file (the RXN header was made manually as well).

Also, we could use OpenBabel to convert separate InChIs (from the RInChI) to SMILES and eventually to add manually stereo in the SMILES if it is lost during the conversion

If you need some assistance in this regards I could help.

[uli-f]

Okay, I did this for a single test and it clearly shows that the addition of the Rauxinfo with 2D coordinates solves the issue. Would you mind merging PR #20.

I would consider this important information for the user of the RInChI functionality in CDK. The best I came up with where to put this was the package.html in the package org.openscience.cdk.rinchi.

I left you a TODO there with regard to the potential loss of stereochemical information. I'd appreciate it if you could amend any documentation there that might be useful in this context.

[ntk73]

PR #20 is merged.

[uli-f]

I added some documentation for this to the package.html, PR #23.

I'd appreciate it if you could carry out any corrections of the documentation in package.html re the stereochemical information and add if there is something missing (tetrahedral stereochemical information?).

Once this is done we can close this issue.

[ntk73]

Meanwhile, I have also added some html documentation :-). It will be needed to resolve some conflicts and then merge your commit from PR #23.

[uli-f]

Meanwhile, I have also added some html documentation :-). It will be needed to resolve some conflicts and then merge your commit from PR #23.

I am sorry, I was on a documentation-writing-roll yesterday! 🏃🏼

I hope it is not too much work to get this merged. There can never be too much documentation! 😄

[uli-f]

Given the merge of PR #23 I am happy to close this one.