From f132abd368fc4c2cd679402fcde220ee81bf05f0 Mon Sep 17 00:00:00 2001
From: Esben Jannik Bjerrum <12825511+EBjerrum@users.noreply.github.com>
Date: Wed, 30 Nov 2022 15:27:53 +0100
Subject: [PATCH] Update test_fptransformers.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rafał Bachorz
  2:57 PM
Hello
@esben
, it seems to me that the SECFP fingerprints are indeed independent from the n_permutations/seed. I have checked the source implementation (https://github.com/reymond-group/mhfp) and it is the case, i.e. the n_permutations/seed is not used (in fact SECFP is called there via static function). What bothers me a bit is that the SECFP calculated with the mentioned above source implementation is different than the one obtained with RDKit (and thus scikit-mol) - presumably for the same settings and the same molecule. I am investigating that.
---
 tests/test_fptransformers.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_fptransformers.py b/tests/test_fptransformers.py
index 84f7196..a570178 100644
--- a/tests/test_fptransformers.py
+++ b/tests/test_fptransformers.py
@@ -186,10 +186,10 @@ def test_SECFingerprintTransformer(chiral_mols_list):
                 'kekulize': True,
                 'length': 1048,
                 'min_radius': 2,
-                #'n_permutations': 2, #TODO, figure out why n_permutations are not influencing
+                #'n_permutations': 2, # The SECFp is not using this setting
                 'radius': 2,
                 'rings': False,
-                #'seed': 1 #TODO, figure out why n_permutations are not influencing
+                #'seed': 1 # The SECFp is not using this setting
                 }
     assert_transformer_set_params(SECFingerprintTransformer, new_params, chiral_mols_list)