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

CanonicalRankAtomsInFragment example in the documentation is not reproducible #5986

Closed
chmnk opened this issue Jan 17, 2023 · 3 comments
Closed

Comments

@chmnk
Copy link
Contributor

chmnk commented Jan 17, 2023

Describe the bug
The documentation's example of CanonicalRankAtomsInFragment produces a different output compared to the documentation.

To Reproduce
In the documentation:

>>>mol = MolFromSmiles('C1NCN1.C1NCN1')
>>>list(CanonicalRankAtomsInFragment(mol, atomsToUse=range(0,4), breakTies=False))
[0,1,0,1,-1,-1,-1,-1]
>>>list(CanonicalRankAtomsInFragment(mol, atomsToUse=range(4,8), breakTies=False))
[-1,-1,-1,-1,0,1,0,1]

My RDKit's behaviour:

>>>mol = Chem.MolFromSmiles('C1NCN1.C1NCN1')
>>>print(list(Chem.CanonicalRankAtomsInFragment(mol, atomsToUse=range(0,4), breakTies=False)))
[4, 6, 4, 6, -1, -1, -1, -1]
>>>print(list(Chem.CanonicalRankAtomsInFragment(mol, atomsToUse=range(4,8), breakTies=False)))
[-1, -1, -1, -1, 4, 6, 4, 6]
>>>print(list(Chem.CanonicalRankAtomsInFragment(mol, atomsToUse=range(0,4), breakTies=True)))
[4, 6, 5, 7, -1, -1, -1, -1]
>>>print(list(Chem.CanonicalRankAtomsInFragment(mol, atomsToUse=range(4,8), breakTies=True)))
[-1, -1, -1, -1, 4, 6, 5, 7]

Expected behaviour
Actually I find the example not very intuitive, cause in both cases breakTies is set to True. So I don't know what behaviour is expected.

Configuration (please complete the following information):

  • RDKit version: both '2023.03.1pre' and '2022.09.3'.
  • OS: CentOS Linux 7
  • Python version: 3.10 and 3.8
  • Are you using conda? Yes
  • If you are using conda, which channel did you install the rdkit from? '2022.09.3' from conda-forge
  • If you are not using conda: how did you install the RDKit? '2023.03.1pre' from sources inside of a conda environment
@greglandrum
Copy link
Member

Confirmed. Thanks for the bug report

@greglandrum greglandrum added this to the 2022_09_5 milestone Jan 17, 2023
@chmnk
Copy link
Contributor Author

chmnk commented Jan 17, 2023

What behaviour is expected by the way? The same as in CanonicalRankAtoms, but with -1 on unlisted atoms, or something else?

@greglandrum
Copy link
Member

greglandrum commented Jan 19, 2023

I think the behavior of the code is fine since the relative ranking of the atoms is correct. The documentation is incorrect and should be fixed.

greglandrum added a commit to greglandrum/rdkit that referenced this issue Feb 9, 2023
greglandrum added a commit that referenced this issue Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants