ebjerrum / scikit-mol Goto Github PK

Integration testing is not implemented in pytest. Adrian has been working on it (Pull request #12 )

Appart from sensible docstrings (#14) some more worked through examples in the notebooks could be helpful, maybe in the form of a "cookbook" taking examples e.g.

• how to create the transformers and a toy dataset and create fingerprints or descriptors.
• How to clean up faulty smiles and standardize the dataset.
• How to create an integrated pipeline and pickle and load it.
• How to use hyperparameter tuning with inbuild sklearn tools
• How to use a third party package such as skopt for tuning

Some of the notebooks cover this a bit, but they could be upgraded.

Some arguments in the fingerprints class seem to be related to molecule (e.g. list of provided atom-invariants, and include_mol). They should maybe not be exposed for a general transformer class?

It would be good with a contribution.md file with information for developers

What to do to add a new transformer
What information to read about base-estimators and transformer mixin from sklearn
Gotcha with the casting to standard types and other tips

Docstring standard

formatting standard?

Some classes are missing docstrings

Hi @cespos , @ap-- , @adrienchaton

I need to get in contact with you regarding a software note. I tried to send an email to the email adresses I could find, but maybe it didn't get through or ended up in the spam filter? Can you please check our slack channel or contact me by mail? (I should be easy to google).

Best Regards
Esben

Some transformer classes are not tested for if .set_params() work as intended in the pytests

Description

Starting from scikit-learn 1.2, it is possible to set the output of transformers to be Pandas objects (Series, DataFrame) instead of the default NumPy arrays: https://scikit-learn.org/stable/auto_examples/miscellaneous/plot_set_output.html#introducing-the-set-output-api
This can be done:

• globally by calling sklearn.set_config(transform_output="pandas"),
• or directly on a Transformer object (or Pipeline) with method set_output(transform="pandas").

Scikit-mol ignores the globally-set option: the output of transformers is still NumPy arrays even after setting it to "pandas".

If set_output is called directly on a scikit-mol transformer object (like SmilesToMolTransformer), we get:

AttributeError: This 'SmilesToMolTransformer' has no attribute 'set_output'

If instead set_output is called on a Pipeline which includes scikit-mol transformer, we get:

ValueError: Unable to configure output for SmilesToMolTransformer() because `set_output` is not available.

Solution

Make scikit-mol transformers compatible with the set_output API.
This can be achieved by defining set_output, get_feature_names_out, and auto_wraps_output_keys on the scikit-mol transformer classes (or their base class, like FpsTransformer). See this for reference.

For instance, the output of a MorganFingerprintTransformer with nBits=1024 could be a DataFrame with column names "fp_morgan_0001", "fp_morgan_0002", ..., "fp_morgan_1024".

Using the library scikit-optimize leads to errors. The optimizer from skopt sets parameters on the transformers as numpy dtypes, e.g. numpy.floats or bools, which leads to errors with the RDKit calls. Likely input sanitatizion and casting to secure dtypes in @Property getters and setters would be a necessary fix.