aiidalab / aiidalab-widgets-base

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

SmilesWidget: Canonicalize SMILES code #507

Closed danielhollas closed 11 months ago

danielhollas commented 11 months ago

Canonicalize user-provided SMILES using RDKit. If the canonical name is different from the input SMILES, we print the canonical SMILES.

Test plan:

  1. Input SMILES C=CC1=C(C2=CC=C(C3=CC=CC=C3)C=C2)C=C(C=C)C(C4=CC=C(C(C=C5)=CC=C5C(C=C6C=C)=C(C=C)C=C6C7=CC=C(C(C=C8)=CC=C8C(C=C9C=C)=C(C=C)C=C9C%10=CC=CC=C%10)C=C7)C=C4)=C1

should give you C=Cc1cc(-c2ccc(-c3ccc(-c4cc(C=C)c(-c5ccc(-c6ccc(-c7cc(C=C)c(-c8ccc(-c9ccccc9)cc8)cc7C=C)cc6)cc5)cc4C=C)cc3)cc2)c(C=C)cc1-c1ccccc1

and should not fail.

I also add a missing error check that caused uncaught exception in #505

Closes #505. Closes #331

codecov[bot] commented 11 months ago

Codecov Report

Patch coverage: 80.00% and project coverage change: +0.02% :tada:

Comparison is base (5038966) 79.42% compared to head (94679cd) 79.44%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #507 +/- ## ========================================== + Coverage 79.42% 79.44% +0.02% ========================================== Files 27 27 Lines 3757 3785 +28 ========================================== + Hits 2984 3007 +23 - Misses 773 778 +5 ``` | [Flag](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507/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/507/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `79.44% <80.00%> (+0.02%)` | :arrow_up: | | [python-3.8](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `79.48% <80.00%> (+0.01%)` | :arrow_up: | | [python-3.9](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `79.48% <80.00%> (+0.01%)` | :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 Changed](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [aiidalab\_widgets\_base/structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL3N0cnVjdHVyZXMucHk=) | `74.96% <71.42%> (-0.19%)` | :arrow_down: | | [tests/test\_structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507?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%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/507/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab)

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

danielhollas commented 11 months ago

@cpignedoli thanks for taking a look. Can you please open a new issue for this new SMILES?

Interestingly, in my application that generates multiple conformers it works. It looks like passing useRandomCoordinates into EmbedMolecule fixes the issue. But let's deal with that in a separate PR.