-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#425 Smiles with attachment points is not read correctly (valences are wrong) #2783
#425 Smiles with attachment points is not read correctly (valences are wrong) #2783
Conversation
Similar bug to #2746; need to remove the old bonds before removing atoms (and thereby recalculating the implicit hydrogens) |
to_remove.push(i); | ||
_bmol->removeBonds(bondsToRemove); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better fix void BaseMolecule::removeAtoms(const Array<int>& indices)
to remove bonds connected to removed atoms?
Because edges without vertex looks weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add some code to remove the edges in removeAtoms, however there are problems. If I put this code after line 813 (_removeAtoms), it doesn't fix this bug. If I put it before this line, then I break a some integration tests.
These tests seem to be about folding/unfolding molecules. There is a comment in Molecule::_removeAtoms about this. I suspect some other parts of the code are relying on having the 'incorrect' number of bonds to an atom.
I think this is a deeper problem that can't be solved with adding code to BaseMolecule::removeAtoms.
"N": "[NH3+]%91.[*:1]%91 |$;_AP1$|", | ||
} | ||
|
||
for key, value in smiles.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such code cause UTs fails due to different sorting in iron python.
To avoid such fails I usually use next code:
for key in sorted(smiles.keys()):
value = smeles[key]
It is unclear without context, but I already hit same issue - it is due to different key order in iron python, so line moved to different position. |
Render at linux and mac could generate files different from windows so you will see fails like
Line after
|
Generic request
#1234 – issue name