Patch/fingerprint tidy 1 #94
Conversation
…easures (see Tanimoto class) expect the bits in sorted order. Without this invariant the CFP similarity is way off par. (Spotted by Roger)
… them and use method2).
…er discrimination.
…t is already there we don't want to take it out but we can avoid the need to do atom typing by using a different aromaticity model.
|
@jonalv, can you please also have a look at it? |
|
Just spotted there is another method already for getting the count fp from features. Will delegate from the util to that.. |
| /** | ||
| * Specific constructor: initializes with descriptor class type, one of ECFP_{p} or FCFP_{p}, where ECFP is | ||
| * for the extended-connectivity fingerprints, FCFP is for the functional class version, and {p} is the | ||
| * path diameter, and may be 0, 2, 4 or 6. |
There was a problem hiding this comment.
"may be 0, 2, 4 or 6.", maybe throw an IllegalArgumentException if not? (ref: http://stackoverflow.com/questions/6086334/is-it-good-practice-to-make-the-constructor-throw-an-exception)
|
Done. |
|
@jonalv any further comments? Would like to push this out for a 1.5.9 release. |
|
I am good. Will admit that I haven't fully grokked all the stuff in the tools and Lingo classes though. |
|
Applying them... |
|
Applied and pushed. |
|
Are these two regressions on Jenkins caused by this patch? org.openscience.cdk.fingerprint.LingoFingerprinterTest.testFingerprint |
|
Yep will patch. |
Some cleanup to fingerprint code.