The IFingerprinter API can be made backwards compatible with CDK 1.4.… #289
Conversation
|
Hi @jonalv, this patch looks good to me, but could you please also glance over it? |
|
@johnmay, there is a merge conflict that you may wish to resolve? |
|
Okay will resolve the conflict |
…x by re-adding and delegating the old method.
johnmay
force-pushed
the
patch/fingerprint-api
branch
from
ef4abff
to
fe745dc
Compare
|
Rebased |
There was a problem hiding this comment.
Without having the complete code in front of me right now ( and being quite rusty at Java by now :) I think this looks like a good thing. I don't remember whether the Signature fingerprint's implementations of the getBitFingerprint methods throws IllegelstateException or not but even if so I don't know if that is an issue really. Using it as BitSet's is going to be painfully memory requesting. The ECFP on folds the bit variant and but not the count variant. Also an interesting approach. Perhaps if this backwards compatibility thing is a big thing for RCDK (I am just guessing here) it could be interesting to create a folded bit signature fingerprint...
|
I'll apply it once the hash test works properly again! |
|
hash test? |
…x by re-adding and delegating the old method.
From writing the benchmarks for CDK 1.4 vs 2.0 it's clear to see where APIs have been broken unnecessarily. This patch re-adds the old IFingerprinter method reducing the barrier to code porting.