-
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
_moltoimg() should honor drawOptions.prepareMolsBeforeDrawing #6792
Merged
greglandrum
merged 3 commits into
rdkit:master
from
ptosco:do-not-prepare-mol-if-requested-not-to
Oct 12, 2023
Merged
_moltoimg() should honor drawOptions.prepareMolsBeforeDrawing #6792
greglandrum
merged 3 commits into
rdkit:master
from
ptosco:do-not-prepare-mol-if-requested-not-to
Oct 12, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… run PrepareMolForDrawing if requested not to
…l-if-requested-not-to
…awing and not call PrepareMolForDrawing if asked not to
greglandrum
approved these changes
Oct 12, 2023
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
greglandrum
added a commit
that referenced
this pull request
Oct 19, 2023
* fixes #6773 * removed unused captures * update release notes and cmakelists for beta (#6788) * Removed some code duplication between Depictor.cpp and common.h (#6368) * - implemented alignOnly mode into RDDepict::generateDepictionMatching2DStructure() - the allowRGroups option now also supports potentially missing R groups (i.e., R groups that do not match any atom, such as those connected to generic aromatic atoms) - added the adjustMolBlockWedging parameter (which defaults to true) to invert/clear molblock wedging information as appropriate - added unit tests for the above new features - added RDDepict::generateDepictionMatching2DStructure() overloads taking RDDepict::ConstrainedDepictionParams parameter for convenience - removed some redundant RDDepict:: namespace specifications * Fix chirality handling when the chiral atom is the first one in a SMARTS (#6730) * Set _SmilesStart when parsing SMARTS. * SmartsWriter should also invert first atoms, like SMILES. * Update test cases now these SMILES match themselves as SMARTS. * rerun bison * cleanup a possible repeated define * When an atom moves from the first to second position winding should flip in SMARTS (i.e. same as SMILES). --------- Co-authored-by: greg landrum <greg.landrum@gmail.com> * Some small cleanups from the UGM Hackathon (#6744) * move definition of a couple global constants from a .h to a .cpp * careful removal of some redundant atom PRECONDITIONS * careful remove of some redundant ROMol PRECONDITIONS a bit of additional cleanup * optimization masquerading as modernization * some more tidying * a bit more atom cleanup * change in response to review * Fixes #6756 (#6780) * update release notes and cmakelists for beta (#6788) * move problematic functions to Chirality namespace * - implemented alignOnly mode into RDDepict::generateDepictionMatching2DStructure() - the allowRGroups option now also supports potentially missing R groups (i.e., R groups that do not match any atom, such as those connected to generic aromatic atoms) - added the adjustMolBlockWedging parameter (which defaults to true) to invert/clear molblock wedging information as appropriate - added unit tests for the above new features - added RDDepict::generateDepictionMatching2DStructure() overloads taking RDDepict::ConstrainedDepictionParams parameter for convenience - removed some redundant RDDepict:: namespace specifications * move problematic functions to Chirality namespace * added missing dependency * let's check what is going wrong * CoordGen tests should not run if CoordGen support is not available in the build --------- Co-authored-by: Tosco, Paolo <paolo.tosco@novartis.com> Co-authored-by: John Mayfield <john@nextmovesoftware.com> Co-authored-by: greg landrum <greg.landrum@gmail.com> * Revert "Removed some code duplication between Depictor.cpp and common.h (#6368)" (#6797) This reverts commit ddfe708. * All 3d des (#6741) * add function to calc all 3D descriptors * fix indentation in CalcMolDescriptors3D * add error handling for 2D molecules * suggested changes * change exception type * add unit tests * add random seed to conf gen in unit tests * fix function call in unit tests * fix expected PMI1 value in tests --------- Co-authored-by: Greg Landrum <greg.landrum@gmail.com> * _moltoimg() should honor drawOptions.prepareMolsBeforeDrawing (#6792) * _moltoimg() should honor drawOptions.prepareMolsBeforeDrawing and not run PrepareMolForDrawing if requested not to * _moltoimg() and _moltoSVG() should honor drawOptions.prepareMolsForDrawing and not call PrepareMolForDrawing if asked not to --------- Co-authored-by: ptosco <paolo.tosco@novartis.com> * Bad lasso (#6751) * Better separation of lines. * Only put atoms in colour list once. * Test. * Hash codes. * Response to review. * First attempt at fixing stray line. * Tidier. * Squared distances. * tag release (#6801) * fixes #6773 * removed unused captures * - fixed comment - fixed bug found in checkIfRingsAreClosed() * - removed redundant NRing queries - added ring size queries to avoid incorrect matches with MCS results originated with MatchFusedRingsStrict * fixed doctests --------- Co-authored-by: ptosco <paolo.tosco@novartis.com> Co-authored-by: Greg Landrum <greg.landrum@gmail.com> Co-authored-by: John Mayfield <john@nextmovesoftware.com> Co-authored-by: Rachael Pirie <56546141+RPirie96@users.noreply.github.com> Co-authored-by: David Cosgrove <davidacosgroveaz@gmail.com>
greglandrum
added a commit
that referenced
this pull request
Nov 10, 2023
* fixes #6773 * removed unused captures * update release notes and cmakelists for beta (#6788) * Removed some code duplication between Depictor.cpp and common.h (#6368) * - implemented alignOnly mode into RDDepict::generateDepictionMatching2DStructure() - the allowRGroups option now also supports potentially missing R groups (i.e., R groups that do not match any atom, such as those connected to generic aromatic atoms) - added the adjustMolBlockWedging parameter (which defaults to true) to invert/clear molblock wedging information as appropriate - added unit tests for the above new features - added RDDepict::generateDepictionMatching2DStructure() overloads taking RDDepict::ConstrainedDepictionParams parameter for convenience - removed some redundant RDDepict:: namespace specifications * Fix chirality handling when the chiral atom is the first one in a SMARTS (#6730) * Set _SmilesStart when parsing SMARTS. * SmartsWriter should also invert first atoms, like SMILES. * Update test cases now these SMILES match themselves as SMARTS. * rerun bison * cleanup a possible repeated define * When an atom moves from the first to second position winding should flip in SMARTS (i.e. same as SMILES). --------- Co-authored-by: greg landrum <greg.landrum@gmail.com> * Some small cleanups from the UGM Hackathon (#6744) * move definition of a couple global constants from a .h to a .cpp * careful removal of some redundant atom PRECONDITIONS * careful remove of some redundant ROMol PRECONDITIONS a bit of additional cleanup * optimization masquerading as modernization * some more tidying * a bit more atom cleanup * change in response to review * Fixes #6756 (#6780) * update release notes and cmakelists for beta (#6788) * move problematic functions to Chirality namespace * - implemented alignOnly mode into RDDepict::generateDepictionMatching2DStructure() - the allowRGroups option now also supports potentially missing R groups (i.e., R groups that do not match any atom, such as those connected to generic aromatic atoms) - added the adjustMolBlockWedging parameter (which defaults to true) to invert/clear molblock wedging information as appropriate - added unit tests for the above new features - added RDDepict::generateDepictionMatching2DStructure() overloads taking RDDepict::ConstrainedDepictionParams parameter for convenience - removed some redundant RDDepict:: namespace specifications * move problematic functions to Chirality namespace * added missing dependency * let's check what is going wrong * CoordGen tests should not run if CoordGen support is not available in the build --------- Co-authored-by: Tosco, Paolo <paolo.tosco@novartis.com> Co-authored-by: John Mayfield <john@nextmovesoftware.com> Co-authored-by: greg landrum <greg.landrum@gmail.com> * Revert "Removed some code duplication between Depictor.cpp and common.h (#6368)" (#6797) This reverts commit ddfe708. * All 3d des (#6741) * add function to calc all 3D descriptors * fix indentation in CalcMolDescriptors3D * add error handling for 2D molecules * suggested changes * change exception type * add unit tests * add random seed to conf gen in unit tests * fix function call in unit tests * fix expected PMI1 value in tests --------- Co-authored-by: Greg Landrum <greg.landrum@gmail.com> * _moltoimg() should honor drawOptions.prepareMolsBeforeDrawing (#6792) * _moltoimg() should honor drawOptions.prepareMolsBeforeDrawing and not run PrepareMolForDrawing if requested not to * _moltoimg() and _moltoSVG() should honor drawOptions.prepareMolsForDrawing and not call PrepareMolForDrawing if asked not to --------- Co-authored-by: ptosco <paolo.tosco@novartis.com> * Bad lasso (#6751) * Better separation of lines. * Only put atoms in colour list once. * Test. * Hash codes. * Response to review. * First attempt at fixing stray line. * Tidier. * Squared distances. * tag release (#6801) * fixes #6773 * removed unused captures * - fixed comment - fixed bug found in checkIfRingsAreClosed() * - removed redundant NRing queries - added ring size queries to avoid incorrect matches with MCS results originated with MatchFusedRingsStrict * fixed doctests --------- Co-authored-by: ptosco <paolo.tosco@novartis.com> Co-authored-by: Greg Landrum <greg.landrum@gmail.com> Co-authored-by: John Mayfield <john@nextmovesoftware.com> Co-authored-by: Rachael Pirie <56546141+RPirie96@users.noreply.github.com> Co-authored-by: David Cosgrove <davidacosgroveaz@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
_moltoimg()
should honordrawOptions.prepareMolsBeforeDrawing
and not runPrepareMolForDrawing
if requested not to.This is annoying when trying to visualize a molecule with original molblock wedging as, for example, chiral Hs were still being added even if one does not want to.