Skip to content

Fixes #5395#5437

Merged
bp-kelley merged 2 commits intordkit:masterfrom
greglandrum:fix/github5395
Jul 20, 2022
Merged

Fixes #5395#5437
bp-kelley merged 2 commits intordkit:masterfrom
greglandrum:fix/github5395

Conversation

@greglandrum
Copy link
Copy Markdown
Member

This is a small change.
It does change the return type of Chem.GetSSSR() in order to make it consistent with Chem.GetSymmSSSR(). I think the consistency is good and that the new return value is more useful.
I added a backwards incompatibility note to the Release Notes about this

@bp-kelley
Copy link
Copy Markdown
Contributor

Looks like a doc test got missed

File "GettingStartedInPython.rst", line 512, in default
Failed example:
Chem.GetSSSR(m)

@greglandrum
Copy link
Copy Markdown
Member Author

Looks like a doc test got missed

File "GettingStartedInPython.rst", line 512, in default Failed example: Chem.GetSSSR(m)

Yep, fixed it

@rwest
Copy link
Copy Markdown
Contributor

rwest commented Dec 9, 2025

The page https://www.rdkit.org/docs/GettingStartedInPython.html#the-sssr-problem still says

Because it is sometimes useful to be able to count how many SSSR rings are present in the molecule, there is a rdkit.Chem.rdmolops.GetSSSR() function, but this only returns the SSSR count, not the potentially non-unique set of rings.

which I guess is no longer true?

I think this is not the only place:

https://github.com/search?q=org%3Ardkit+%22not+the+potentially+non-unique+set+of+rings.%22&type=code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants