Fixes #7375 #7381
Fixes #7375 #7381
Conversation
|
My initial reaction is to disagree with the changes to require a particular level of ring finding. I appreciate that this makes it harder for people to generate misleading results, but we've always provided the option for client-code to skip (or approximate) parts of sanitization which may be time consuming. Doing this assumes that the author of the client code understands and is thinking about what they are doing and increase the odds that they will generate incorrect results. I think it's worth talking about changing this "you can mess with the defaults, but you then should know what you are doing" approach, but it is a longer conversation and should result in a separate issue/PR that we can then point to. Would you mind reducing this PR to just the bug fix and then we can have the conversation about the ring finding elsewhere? |
Sure |
Fixes #7375
The fix is easy: just add a check ring info (and the proper initialization if not found) in the second condition in
KekulizeFragment().Besides that, this PR does something I've wanted to do since #6903 added specific checks for the different levels of ring info: updating the ring info checks in several code locations to check for the proper ring info level instead of just checking for initialization.
This last change broke some tests because it changed the canonicalization of some strings. This should make canonicalization more robust, since now it shouldn't depend on the ring info present on the input molecule anymore.