Issue with is.in.ring - Githubissues

vjindyan commented 4 years ago

The is.in.ring function does not seem to be working as intended. It returns FALSE for all atoms in SMILES C1OC1, c1ccccc1, c1ccccc1N. Example: mol <- parse.smiles("c1ccccc1")[[1]] atms <- get.atom.count(mol) atyp <- get.atoms(mol) cmat <- get.connection.matrix(mol) bnds <- get.bonds(mol)

atts <- matrix(0, atms, 5)
for ( i in 1:atms) { atts[i,1] <- get.atom.index(atyp[[i]], mol)+1 atts[i,2] <- get.symbol(atyp[[i]]) atts[i,3] <- get.hydrogen.count(atyp[[i]]) atts[i,4] <- is.aromatic(atyp[[i]]) atts[i,5] <- is.in.ring(atyp[[i]]) } atts [,1] [,2] [,3] [,4] [,5]
[1,] "1" "C" "1" "TRUE" "FALSE" [2,] "2" "C" "1" "TRUE" "FALSE" [3,] "3" "C" "1" "TRUE" "FALSE" [4,] "4" "C" "1" "TRUE" "FALSE" [5,] "5" "C" "1" "TRUE" "FALSE" [6,] "6" "C" "1" "TRUE" "FALSE"

zachcp commented 4 years ago

Vijay,

Try this:

library(rcdk)

mol  <- parse.smiles("c1ccccc1N")[[1]]
mol  <- generate.2d.coordinates(mol)

atoms    <- get.atoms(mol)
atmcount <- get.atom.count(mol)
atts <- matrix(0, atmcount, 5)

 for (i in 1:atmcount) {
     atts[i,1] <- get.atom.index(atoms[[i]], mol)+1
     atts[i,2] <- get.symbol(atoms[[i]])
     atts[i,3] <- get.hydrogen.count(atoms[[i]])
     atts[i,4] <- is.aromatic(atoms[[i]])
     atts[i,5] <- is.in.ring(atoms[[i]])
 }

> atts
     [,1] [,2] [,3] [,4]    [,5]
[1,] "1"  "C"  "1"  "TRUE"  "TRUE"
[2,] "2"  "C"  "1"  "TRUE"  "TRUE"
[3,] "3"  "C"  "1"  "TRUE"  "TRUE"
[4,] "4"  "C"  "1"  "TRUE"  "TRUE"
[5,] "5"  "C"  "1"  "TRUE"  "TRUE"
[6,] "6"  "C"  "0"  "TRUE"  "TRUE"
[7,] "7"  "N"  "2"  "FALSE" "FALSE"

Note that I add the line mol <- generate.2d.coordinates(mol)

rajarshi commented 4 years ago

Thats wierd that it requires 2D coordinates for it to work

rajarshi commented 4 years ago

The better way to address is to perform ring perception. With the latest code you can do this

> mol <- parse.smiles('c1ccccc1')[[1]]
> sapply(get.atoms(mol), is.in.ring)
[1] FALSE FALSE FALSE FALSE FALSE FALSE
> cycles <- J("org.openscience.cdk.graph.Cycles", "essential", mol)
> cycles$markRingAtomsAndBonds(mol)
[1] 1
> sapply(get.atoms(mol), is.in.ring)
[1] TRUE TRUE TRUE TRUE TRUE TRUE

CDK-R / cdkr

Issue with is.in.ring #97