search

Meredith-Lab / volcalc

volcalc: Calculate Volatility of Chemical Compounds
https://meredith-lab.github.io/volcalc/
Other
5 stars 1 forks source link

Validate compounds by looking for the "symptoms" of OpenBabel issues #104

Closed Aariq closed 6 months ago

Aariq commented 8 months ago

Addresses #85 (and also closes #56) by checking the output of ChemmineR::propOB() for the symptoms of OpenBabel issues parsing SDFs. Why propOB()? Simply because that is the only ChemmineR function we have found that actually complains about problems in .mol files.

Currently this is tested with two cases:

I'm happy to have this PR merged as-is or add additional tests—I'm sure there are other ways that inputs (.mol files or SMILES) can be just broken enough to trigger OpenBabel errors, but not R errors, and return incorrect results.

Unfortunately, since InChI can't be generated with the windows version of ChemmineOB, this validation option can't be used on Windows. This is documented in the help file for get_fx_groups() and a warning is printed when validate = TRUE is used on Windows.

Still to do?:

Aariq commented 8 months ago

It seems like the built-in version of OpenBabel included in the windows version of ChemmineOB might behave differently when it comes to creating the formula and InChI that we are using to diagnose parsing issues. Need to add some tests and investigate.

Aariq commented 8 months ago

It looks like this strategy doesn't currently work on Windows because the windows version of ChemmineR::propOB() doesn't generate InChI. I've opened an issue: https://github.com/girke-lab/ChemmineOB/issues/37.

Aariq commented 8 months ago

ChemmineOB can't generate InChI on Windows https://github.com/girke-lab/ChemmineOB/issues/37#issuecomment-2008364855

For now I may just only allow the validate argument to work on non-windows operating systems.

Aariq commented 7 months ago

ChemmineOB currently is encountering errors when being built from source on macOS: https://github.com/girke-lab/ChemmineOB/issues/35.

Aariq commented 6 months ago

Ok, going to go ahead and merge despite checks on macOS failing since I've confirmed checks work locally on macOS with binary version of ChemmineOB. Might do a separate PR to force GH actions to install ChemmineOB from binary rather than source if I can figure out how to do that easily.