-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add scikit-fingerprints #359
Comments
thank you for taking this on, summarizing our earlier conversation
|
its strange that but in here I see that the value corresponding to SMILES are validated with a different logic in wouldnt |
@Scienfitz I ran
One more question about the test - do I need to run them separately in env where CHEM is not installed? |
Also, do I need to do sth to re-generate documentation svgs or will it be done automatically? I guess |
please can you use will probably give you the same error but to exclude that its any environment misconfiguration I have a suspicion for the first error, but impossible to help without seeing the code. You can already open the PR in draft mode |
dont care about the pictures at this moment, they actually shouldnt change much if the fingerprints from the package are implemented identically |
Regarding pictures: Once everything else is fixed, just ping me about the pictures @Hrovatin . I can then give you a heads-up/we can discuss how to update pictures, but as Martin says, this is not really relevant at the moment. |
Test results. For mypy I need to do a few updates and will add once finished.
|
For mypy I have multiple issues with
|
I think rdkit is a main dep of skfp so we do not have to decide and can keep all other funcs
Is ideally designed to coincide with the namign scheme ie dropping capitalization and
Yes for now
not sure what you mean but Adrian raised the one point: If we generate the enums automatically, would that destroy the tab completion when I type |
Regarding Errors
I suspect the missing dtype cast messes with the size estimation vs actual size in the memory test. E.g. if a fingeprrint returns some of their columns as int the estimation that these are all floats32 doesnt hold anymore
Ingore, they appear 40% of the time at random
No clear idea. Seems like the overall contruction of the parameter computational representation |
Replace mordred and rdkit fingerprints with scikit-fingerpints and enable other fingerprints from the package. Aim to remove rdkit and mordred install.
Dev in: https://github.com/Hrovatin/baybe/tree/feature/scikit_fingerprints
Notes/Discuss:
is_valid_smiles
: not used anywhereget_canonical_smiles
mordred
check inedbo
- can this be used for any fingeprint (before was mordred and rdkit)The text was updated successfully, but these errors were encountered: