-
Notifications
You must be signed in to change notification settings - Fork 845
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
update avalontools version to incorporate bug fixes #6513
update avalontools version to incorporate bug fixes #6513
Conversation
…e is out of bounds - removed 0-padding ahead of parsing bond line as it is unnecessary and potentially harmful - forked Avalon so it can be patched more conveniently
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.
Unsurprisingly I agree with using a fork for the avalontools code.
Do you think it would make sense to have the fork live in the RDKit organization?
@greglandrum absolutely. I did not want to do that without asking while you were on holiday, but since you are now proposing this I will go ahead and move it under https://github.com/rdkit. |
…of-bounds-atom-idx
updated checksum and release
@greglandrum Done. |
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.
LGTM
* - fixed a crash in Avalon occurring when an atom idx in the bond table is out of bounds - removed 0-padding ahead of parsing bond line as it is unnecessary and potentially harmful - forked Avalon so it can be patched more conveniently * updated the Avalon pre-relase number and updated the checksum * updated pre-release number and checksum * moved the Avalon fork under RDKit organization updated checksum and release --------- Co-authored-by: Tosco, Paolo <paolo.tosco@novartis.com>
This PR fixes a crash in Avalon occurring when an atom idx in the bond table is out of bounds.
I also decided to fork Avalon as previously discussed with @greglandrum. This allows to patch it more conveniently and avoid using horrible regular expressions in CMake files.
Finally, I added a couple of unit tests RDKit-side to test the fixes.