Skip to content
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

non-flatness check #54

Merged
merged 8 commits into from
Mar 4, 2025
Merged

non-flatness check #54

merged 8 commits into from
Mar 4, 2025

Conversation

chmnk
Copy link
Contributor

@chmnk chmnk commented Mar 3, 2025

Addresses this issue #53

Adds a non-flatness check for non-flat 5- and 6-membered rings with the following SMARTS patterns

    "nonaromatic_5_membered_rings": "[C,O,S,N&!a]~1[C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a]1",
    "nonaromatic_6_membered_rings": "[C,O,S,N&!a]~1[C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a]1",
    "nonaromatic_6_membered_rings_db03_0": "[C&!a]~1[C&!a][C,O,S,N&!a]~[C,O,S,N&!a][C&!a][C&!a]1",
    "nonaromatic_6_membered_rings_db03_1": "[C&!a]~1[C&!a][C&!a]~[C&!a][C,O,S,N&!a][C&!a]1",
    "nonaromatic_6_membered_rings_db02_0": "[C&!a]~1[C&!a][C&!a][C,O,S,N&!a]~[C,O,S,N&!a][C&!a]1",
    "nonaromatic_6_membered_rings_db02_1": "[C&!a]~1[C&!a][C,O,S,N&!a][C&!a]~[C&!a][C&!a]1",

@maabuu maabuu linked an issue Mar 4, 2025 that may be closed by this pull request
@maabuu maabuu added the enhancement New feature or request label Mar 4, 2025
@maabuu
Copy link
Owner

maabuu commented Mar 4, 2025

Could you merge the latest main branch into the PR?

The tests failed due to a deprecated version of actions/upload-artifact. The main branch is now on the latest version.

@maabuu
Copy link
Owner

maabuu commented Mar 4, 2025

The SMARTS do not seem completely correct yet. Could the &!a be dropped?
smarts.plus shows the second and third SMARTS as:
smartsview-1
smartsview-2

@maabuu
Copy link
Owner

maabuu commented Mar 4, 2025

If I understand SMARTS correctly, capital letters match only aliphatic atoms and hence we can drop the &!a. How about these?

    "nonaromatic_5_membered_rings": "[C,O,S,N]~1[C,O,S,N][C,O,S,N][C,O,S,N][C,O,S,N]1",
    "nonaromatic_6_membered_rings": "[C,O,S,N]~1[C,O,S,N][C,O,S,N][C,O,S,N][C,O,S,N][C,O,S,N]1",
    "nonaromatic_6_membered_rings_db03_0": "[C]~1[C][C,O,S,N]~[C,O,S,N][C][C]1",
    "nonaromatic_6_membered_rings_db03_1": "[C]~1[C][C]~[C][C,O,S,N][C]1",
    "nonaromatic_6_membered_rings_db02_0": "[C]~1[C][C][C,O,S,N]~[C,O,S,N][C]1",
    "nonaromatic_6_membered_rings_db02_1": "[C]~1[C][C,O,S,N][C]~[C][C]1",

@chmnk
Copy link
Contributor Author

chmnk commented Mar 4, 2025

capital letters match only aliphatic atoms

Good to know, I didn't check it. Updated the PR

Copy link
Owner

@maabuu maabuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@maabuu maabuu merged commit 3052249 into maabuu:main Mar 4, 2025
16 checks passed
@maabuu
Copy link
Owner

maabuu commented Mar 4, 2025

Thank you very much for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check non-flatness
2 participants