Hi,
I would also go for the second option (i.e. replace the current SMART): I
also see it as a bug fix. What you could do is to highlight in the release
notes or somewhere else the call one would have to do to mimic the behavior
of previous releases:
Lipinski.RotatableBondSmarts =
Chem.MolFromSmarts('[!$(*#*)&!D1]-&!@[!$(*#*)&!D1]')
Lipinski.NumRotatableBonds(m)
Grégori
Date: Fri, 31 Jan 2014 05:05:56 +0100
> From: Greg Landrum <greg.land...@gmail.com>
> Subject: [Rdkit-discuss] Possible rotatable bonds replacement
> To: RDKit Discuss <rdkit-discuss@lists.sourceforge.net>, Toby
> Wright
> <toby.wri...@inhibox.com>
> Message-ID:
> <CAD4fdRTpTwHbq9iC3VYKfTKOZeGEtsjFG=
> xpfjmyue6x0_p...@mail.gmail.com>
> Content-Type: text/plain; charset="iso-8859-1"
>
> Dear all,
>
> A question for the community:
>
> Toby Wright submitted a pull request this week that introduces a new,
> stricter, rotatable bond definition:
> https://github.com/rdkit/rdkit/pull/211/files
> The new SMARTS, re-formatted to be somewhat more readable, is:
> [!$(*#*)&\
> !D1&\
> !$(C(F)(F)F)&\
> !$(C(Cl)(Cl)Cl)&\
> !$(C(Br)(Br)Br)&\
> !$(C([CH3])([CH3])[CH3])&\
> !$([CD3](=[N,O,S])-!@[#7,O,S!D1])&\
> !$([#7,O,S!D1]-!@[CD3]=[N,O,S])&\
> !$([CD3](=[N+])-!@[#7!D1])&\
> !$([#7!D1]-!@[CD3]=[N+])]\
> -!@\
> [!$(*#*)&\
> !D1&\
> !$(C(F)(F)F)&\
> !$(C(Cl)(Cl)Cl)&\
> !$(C(Br)(Br)Br)&\
> !$(C([CH3])([CH3])[CH3])]
>
> Toby was quite careful and added a new descriptor -
> NumStrictRotatableBonds() - that uses this SMARTS.
>
> I see a few options to deal with this:
>
> I could add the new descriptor as Toby provided it. People are then free to
> pick between NumRotatableBonds() and NumStrictRotatableBonds(). This has
> the advantage of maintaining strict backwards compatibility, but I could
> imagine it being confusing/irritating to people using the code to have to
> choose between them (or, worse, using both).
>
> Another option is to just replace the current NumRotatableBonds() SMARTS
> with the new one.
> This loses backwards compatibility, but replaces NumRotableBonds() with
> something more correct.
>
> Finally, I could take a hybrid approach: replace the default
> NumRotatableBonds() with the new one, but add an extra argument that allows
> the old one to be used.
>
> I'm leaning towards the second option. I'd normally go with the third, but
> I almost view this as a bug fix for the rotatable bonds definition.
>
> Comments? suggestions? Other options?
> -greg
>
------------------------------------------------------------------------------
WatchGuard Dimension instantly turns raw network data into actionable
security intelligence. It gives you real-time visual feedback on key
security issues and trends. Skip the complicated setup - simply import
a virtual appliance and go from zero to informed in seconds.
http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk
_______________________________________________
Rdkit-discuss mailing list
Rdkit-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rdkit-discuss
