MichelML commented 1 year ago

Checklist:

[x] Add tests to cover the fixed bug(s) or the new introduced feature(s) (if appropriate).
[x] Update the API documentation is a new function is added or an existing one is deleted.
[x] Added a news entry.
- copy news/TEMPLATE.rst to news/my-feature-or-branch.rst) and edit it.

simple addition here, already discussed with @maclandrol , let me know if anything is missing, otherwise feel free to merge.

It uses the same function as ecfp but with different default args.

Also seems like a reformatting is needed, see PR needing to be merged in this one #165

codecov[bot] commented 1 year ago

Codecov Report

Merging #164 (8d28370) into main (6571e70) will decrease coverage by 0.02%. The diff coverage is 100.00%.

:exclamation: Current head 8d28370 differs from pull request most recent head c2fd9e9. Consider uploading reports for the commit c2fd9e9 to get more accurate results

@@            Coverage Diff             @@
##             main     #164      +/-   ##
==========================================
- Coverage   91.21%   91.20%   -0.02%     
==========================================
  Files          47       47              
  Lines        3507     3500       -7     
==========================================
- Hits         3199     3192       -7     
  Misses        308      308

Flag	Coverage Δ
unittests	`91.20% <100.00%> (-0.02%)`	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
datamol/align.py	`100.00% <ø> (ø)`
datamol/conformers/_conformers.py	`93.16% <ø> (ø)`
datamol/convert.py	`93.01% <ø> (ø)`
datamol/descriptors/compute.py	`100.00% <ø> (ø)`
datamol/descriptors/descriptors.py	`90.90% <ø> (-1.30%)`	:arrow_down:
datamol/fp.py	`84.50% <ø> (ø)`
datamol/fragment/_assemble.py	`72.40% <ø> (ø)`
datamol/io.py	`91.97% <ø> (ø)`
datamol/isomers/_enumerate.py	`100.00% <ø> (ø)`
datamol/log.py	`100.00% <ø> (ø)`
... and 10 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

MichelML commented 1 year ago

FAILED tests/test_fp.py::test_all_fps - AttributeError: module 'numpy' has no attribute 'float'.
`np.float` was a deprecated alias for the builtin `float`. To avoid this error in existing code, use `float` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.float64` here.

seems unrelated to this PR and likely related to a numpy update, np.float was removed since numpy 1.24.0, see release notes here https://github.com/numpy/numpy/releases/tag/v1.24.0

hadim commented 1 year ago

I don't see any np.float in the datamol code so ut might be due to an upstream dep (rdkit?). I can look into it tomorrow.

MichelML commented 1 year ago

yes @hadim it is surely it , doing a test run to confirm

hadim commented 1 year ago

The tests are only failing for rdkit ≤2021.09 which make sense (newer numpy versions are just incompatible with those rdkit versions). Not sure what's the best thing to do here since the issue is with an upstream dep (beside not supporting rdkit≤2021).

I'll follow up soon. (how urgent is that PR?).

MichelML commented 1 year ago

@hadim not urgent, and we're moving away from 2021 rdkit on kernel and lambdomics, once my latest PR is merged I'd say you could remove support for that version in datamol. I'll let you know when you can if that's good. Should be this week or early next week.

hadim commented 1 year ago

Ok sounds good (no rush on my side).

hadim commented 1 year ago

@MichelML I dropped the tests for rdkit<2022. All the other changes are not directly related to that PR (sorry for the mix).

I'll merge once green here.

Thanks!

datamol-io / datamol

Add missing fcfp func in fingerprint functions dict #164

Codecov Report