Skip to content

Add 'k' extension to SMARTS to support ringsize queries#9172

Merged
greglandrum merged 6 commits intordkit:masterfrom
greglandrum:feat/ringsmarts
Mar 17, 2026
Merged

Add 'k' extension to SMARTS to support ringsize queries#9172
greglandrum merged 6 commits intordkit:masterfrom
greglandrum:feat/ringsmarts

Conversation

@greglandrum
Copy link
Copy Markdown
Member

@greglandrum greglandrum commented Mar 14, 2026

The r primitive in SMARTS is for minimum SSSR ring size, so for this molecule:
image
we get results like this:

In [5]: m = Chem.MolFromSmiles('C1CC2N1CCCCCC2')

In [6]: m.GetSubstructMatches(Chem.MolFromSmarts('[r8]'))
Out[6]: ((4,), (5,), (6,), (7,), (8,), (9,))

i.e. atoms 2 and 3 do not match the r8 search; they would match an r4 search:

In [3]: m.GetSubstructMatches(Chem.MolFromSmarts('[r4]'))
Out[3]: ((0,), (1,), (2,), (3,))

This PR adds the new primitive k that just uses SSSR ring size:

In [7]: m.GetSubstructMatches(Chem.MolFromSmarts('[k8]'))
Out[7]: ((2,), (3,), (4,), (5,), (6,), (7,), (8,), (9,))

This is part of fixing #9143, but I think it will be generally useful.

@greglandrum greglandrum added this to the 2026_03_1 milestone Mar 14, 2026
@greglandrum greglandrum requested a review from ricrogz March 14, 2026 05:22
Copy link
Copy Markdown
Contributor

@ricrogz ricrogz left a comment

Choose a reason for hiding this comment

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

LGTM. Just a small suggestion to use an enum instead of a string.

Comment thread Code/GraphMol/QueryOps.cpp Outdated
Queries::Query<int, Atom const *, true> *query, Atom const *) {
std::string descr = query->getDescription();

std::string prefix;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given the limited number of choices here, and that the prefix is never shown to the user, would it make sense to use an enum class for this, like:

enum class : char  { EQUAL, LESS, GREATER, RANGE };

@greglandrum
Copy link
Copy Markdown
Member Author

LGTM. Just a small suggestion to use an enum instead of a string.

Makes sense. The suggested changes are in

@greglandrum greglandrum merged commit 972b31e into rdkit:master Mar 17, 2026
12 checks passed
@greglandrum greglandrum deleted the feat/ringsmarts branch March 17, 2026 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants