Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-18 Thread Giovanni Cincilla
Very good! I'm happy to hear that. Cheers On Thu, Feb 18, 2016 at 6:44 PM, John M wrote: > Hopefully soon, I'm in the process of moving house this/next month but one > that's settled I'll make it ASAP. > > John > > Regards, > John W May > john.wilkinson...@gmail.com

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-18 Thread John M
Hopefully soon, I'm in the process of moving house this/next month but one that's settled I'll make it ASAP. John Regards, John W May john.wilkinson...@gmail.com On 18 February 2016 at 16:40, Giovanni Cincilla wrote: > Guys, > Thanks for your quick answers. More than

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-18 Thread Giovanni Cincilla
Guys, Thanks for your quick answers. More than quick they were in "real-time"! Amazing ;-) OK, so if we base on the Javadocs definition where "a chain exists only if there are two or more atoms" then the current implementation is correct. This is great as you solved all the inconsistencies I

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-18 Thread John M
It's a simple change but it makes sense to me a chain is length>1 John Regards, John W May john.wilkinson...@gmail.com On 18 February 2016 at 14:52, Giovanni Cincilla wrote: > Hi guys, > I just had the possibility to test the corrected version. In general the >

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-18 Thread Giovanni Cincilla
Hi guys, I just had the possibility to test the corrected version. In general the inconsistencies were solved, but I still have doubt. Molecules from Row2 to Row8 now have all the correct values for LargestChainDescriptor. Anyway I think that Row0 and Row1 molecules should have (given its

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-14 Thread Giovanni Cincilla
OK, thank you for the clarification! Cheers On Fri, Feb 12, 2016 at 8:52 PM, John M wrote: > Hi Gio, > > Next release is 1.5.13. Probably do it soon, not a huge number of changes > but I know a few patches people are waiting (this now being added as one of > them).

[Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Giovanni Cincilla
Dear all, I use CDK mainly through KNIME and I found some supposed inconsistencies using the LargestChainDescriptor. I originally posted my doubt in KNIME-CDK forum where I also provided examples: https://tech.knime.org/forum/cdk/cdk-largest-chain-descriptor-inconsistencies#comment-41502 I'm not

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
Could you provide an example where issues 2 & 3 show up? The descriptor is meant to compute the length of the longest aliphatic chain in a molecule On Thu, Feb 11, 2016 at 4:29 AM, Giovanni Cincilla wrote: > Dear all, > I use CDK mainly through KNIME and I found some

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
Yes, it does appear to be a bug. Inspeting the code the LargestChainDescriptor is looking for the longest path that contains non-aromatic, non-ring atoms. For Row0, there are only two non-ring, non-aromatic atoms and so there are two possible chains with a single atom, hence each chain has the

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
you can ignore the patch since I've just made a pull request with this fix On Thu, Feb 11, 2016 at 10:47 AM, Rajarshi Guha wrote: > Yes, it does appear to be a bug. > > Inspeting the code the LargestChainDescriptor is looking for the longest > path that contains

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread John M
Cheers for patch Rajarshi, added some clean up the code: https://github.com/cdk/cdk/pull/195 but get a test failure on, "CC=CC(C)=O" expected longest path is 6 but I get 5? 5 looks correct to me? John Regards, John W May john.wilkinson...@gmail.com On 11 February 2016 at 17:03, Rajarshi Guha

Re: [Cdk-user] Clarification about LargestChainDescriptor behavior

2016-02-11 Thread Rajarshi Guha
well the first looks at all atoms, the other only considers carbon chains. I'd say keep them both in for now (though the first could be parametrized on element, in which case we'd only need one) On Thu, Feb 11, 2016 at 1:06 PM, John M wrote: > Hmm.. should we