aiidalab / aiidalab-widgets-base

Reusable widgets for AiiDAlab applications
MIT License
6 stars 17 forks source link

Remove openbabel dependency #518

Closed danielhollas closed 11 months ago

danielhollas commented 11 months ago

As discussed in https://github.com/aiidalab/aiidalab-docker-stack/issues/388 we want to remove openbabel dependency from AWB because it cannot be installed by pip. We use it here only as a fallback to RDKit in SmilesWidget.

In my experience, it is not needed once the RDkit SMILES generation is more stable. @cpignedoli I would be interested if you have any SMILES that needed Openbabel. Please open a separate issue if you have any that still don't work for the current code. Thanks!

This also includes a little cleanup of the canonicalization code.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a1b5134) 79.39% compared to head (528e74b) 79.98%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #518 +/- ## ========================================== + Coverage 79.39% 79.98% +0.59% ========================================== Files 27 27 Lines 3849 3828 -21 ========================================== + Hits 3056 3062 +6 + Misses 793 766 -27 ``` | [Flag](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [python-3.10](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `79.98% <90.47%> (+0.59%)` | :arrow_up: | | [python-3.8](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `80.02% <90.47%> (+0.59%)` | :arrow_up: | | [python-3.9](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `80.02% <90.47%> (+0.59%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [tests/test\_structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-dGVzdHMvdGVzdF9zdHJ1Y3R1cmVzLnB5) | `100.00% <100.00%> (ø)` | | | [aiidalab\_widgets\_base/structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/518?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL3N0cnVjdHVyZXMucHk=) | `77.99% <83.33%> (+3.10%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cpignedoli commented 11 months ago

Good point @danielhollas indeed I do not find anymore cases that justify openbabel so from my side we can remove it and if I find some new problematic cases we can think about how to handle them.

I tested an old molecule I had that gave me problems but also with openbabel seems not to work.

I add it here as another problematic case not working at present: [H][C]1C2=CC=CC3=C2C(C4=C1C=CC5=C4C6=C(C7=C([C]([H])C8=CC=CC9=CC=CC7=C98)C=C%10)C%10=CC=C6C=C5)=CC=C3

a correct (not best conformer) interpretation of this helicoidal molecule is provided by Mathematica:

image