Skip to content

Move Sgroups functionality off of ROMol#1

Merged
ricrogz merged 7 commits intoricrogz:sgroups_prfrom
greglandrum:sgroups_pr
Nov 28, 2018
Merged

Move Sgroups functionality off of ROMol#1
ricrogz merged 7 commits intoricrogz:sgroups_prfrom
greglandrum:sgroups_pr

Conversation

@greglandrum
Copy link
Copy Markdown

This moves the public interface off of ROMol and into free functions.

@ricrogz ricrogz requested a review from d-b-w November 21, 2018 14:45
Copy link
Copy Markdown
Collaborator

@d-b-w d-b-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally like this reorganization. I particularly like that it removes the expansion of the Mol interface.

When I read your comment in the rdkit repo PR, I thought you were considering some type of even more general data structure like a map of:

std::map<std::vector<Atom>, RDProps> d_atom_groups;

which could hold SGroups, stereo groups, PDB Residue information, etc. I guess that seems like excessive complexity/flexibility.

@ricrogz ricrogz merged commit 34117b7 into ricrogz:sgroups_pr Nov 28, 2018
@ricrogz
Copy link
Copy Markdown
Owner

ricrogz commented Nov 28, 2018

Ok, after commenting with @d-b-w, we have decided to make the following changes to this, and update the PR to rdkit/rdkit:

  • Remove getNextFreeSGroupId, since it is broken and is not really required.
  • Re-add the comment about using two loops in copying SGroups to a new ROMol.
  • clang-format changed files.
  • Update to current rdkit master.

ricrogz pushed a commit that referenced this pull request Mar 20, 2023
…it#5991)

* Add CHEMBL filters to the filter catalog

* Add catalogs to the top level, add basic tests

* Remove explicit hydrogens in queries
Changes:
   acyclic imine:
        C-!@[NX2]=[C!R,#1]-C ->
        C-!@[NX2]=[C!R]-C
        hypervalent hydrogen dropped

   Filter5_azo:
        [!#7,#1]~[NX2R0]=[NX2R0]~[!#7,#1] =>
        [!#7]~[NX2R0]=[NX2R0]~[!#7]
        X is explicit connections, don't need explicit hydrogen

   Filter18_oxime_ester:
       [!#7,#1]C=NO[C,S,P](=O)* ->
       [!#7][CX3]=NO[C,S,P](=O)*
       Adding explicit connections takes care of #1

   Filter30_beta_halo_carbonyl:
        [#6,#1]C(=O)CC[Br,I] ->
        [$(C(-C)(-C)(=O)),$([CH](=O)-C)](=O)CC[Br,I]
        Make explicit recursive smarts for the #6,#1

* Remove std::cout

* Add a basic test that we can get the CHEMBL filters

* Add CHEMBL filters to wrappers

* Fix a dropped paren

* Fix capitalization

* Fix more hydrogen or queries

* Finish removing hydrogens in ors, add tests

* Separate the filter files for readability

* Add missing zinc.in

* Remove Filters.cpp.in

---------

Co-authored-by: Brian Kelley <bkelley@relaytx.com>
ricrogz pushed a commit that referenced this pull request Feb 1, 2024
…rocycles (rdkit#7032)

* Fix macrocycle bounds calculation, and make it accessible from Python

* Previously, C=C([#1])-C([#1])=C groups in a macrocycle were always
  cis. Although the bounds within the group allowed cis or trans, the
  1-4 bounds of the H atoms were always set to trans, which forces the
  carbons into the cis position. This change fixes this behavior to
  allow cis or trans conformations
* Previously, the useMacrocycle14config flag could only be used from
  Python by creating an ETKDGv3 object, and defaulted to False
  otherwise. This change adds the flag to EmbedMolecule,
  EmbedMultipleConfs, and the EmbedParameters Python interface.
* The new default for useMacrocycle14config is True to be consistent
  with the default for useMacrocycleTorsions. Since the old default was
  False, this could change the output of existing scripts.

* Add test case for updated macrocycle distance bounds

* Remove 2 unused variables

* Fix a python test that set .seed instead of .randomSeed on an ETKDGv3

---------

Co-authored-by: Franz Waibl <waiblfranz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants