mojaie / MolecularGraph.jl

Graph-based molecule modeling toolkit for cheminformatics
MIT License
189 stars 27 forks source link

sethighlight! doesn't work on single carbon atoms in skeletal notation #82

Closed Boxylmer closed 1 year ago

Boxylmer commented 1 year ago

MWE:

using MolecularGraph
function drawmol(mol; draw_indices=false, highlight_indices=missing)
    nohmol = removehydrogens(mol; all=false)
    canvas = SvgCanvas()
    draw2d!(canvas, nohmol; )
    if draw_indices
        drawatomindex!(canvas, mol; )
    end
    if !ismissing(highlight_indices)
        subgraph = nodesubgraph(mol, highlight_indices)
        sethighlight!(canvas, subgraph)
    end
    molsvg = tosvg(canvas, 300, 300)
    return molsvg
end

mol = smilestomol("CC(C)CCCC(C)C1CCC2C1(CCC3C2CC=C4C3(CCC(C4)O)C)C")
res = drawmol(mol; highlight_indices = [3, 4, 5, 24], draw_indices=true)

image

Notice that indices 3 and 24 were not highlighted due to them not spanning an edge, nor having an element symbol present to highlight.

Would it be worthwhile to add some code to cover this edge case? I'm not familiar with how setting highlights works to know it's easy to set it for only a junction here.

function sethighlight!(
        canvas::Canvas, substr::UndirectedGraph; color=Color(253, 216, 53))
    isatomvisible_ = isatomvisible(substr)
    for i in edgeset(substr)
        (u, v) = getedge(substr, i)
        setbondhighlight!(canvas, Segment{Point2D}(canvas.coords, u, v), color)
    end
    for i in nodeset(substr)
        isatomvisible_[i] || continue
        pos = Point2D(canvas.coords, i)
        setatomhighlight!(canvas, pos, color)
    end
    return
end
mojaie commented 1 year ago

Thank you very match for catching. As you mentioned, it doesn't work with carbon chain now. Fortunately setatomhighlight! (in draw/svg.jl) only depends on canvas.fontsize setting and atom position, so a possible workaround is just to remove isatomvisible_[i] || continue (so also isatomvisible_ = isatomvisible(substr) seems unnecessary). If it works, I would appreciate If you would like to make a pull request.

Boxylmer commented 1 year ago

I've never actually done that before! I'll give it a shot. Thanks for the response!

Boxylmer commented 1 year ago

Fantastic! Looks like the issue is solved.