MolChemicalFeature.GetPos() returns value for molecule's default conformer

[greglandrum]

Description:

Reported on the mailing list here: https://www.mail-archive.com/rdkit-discuss@lists.sourceforge.net/msg09001.html

The python wrapper needs an overload of MolChemicalFeature.GetPos() that takes no arguments (instead of the current code, which defaults the confId to -1).

[MaxGreil]

Hi @greglandrum ,
I would like to take on this issue. Can you please explain what needs to be done here?

[greglandrum]

Hi @MaxGreil. Thanks for volunteering.
Let's start with the basics: are you a C++ programmer? This is a change that needs to be made in the C++ code that provides the RDKit's Python interface and if you aren't familiar with working C++ this is going to be quite tricky.

[MaxGreil]

Hi @greglandrum,
I'm an advanced beginner/intermediate C++ programmer.

[greglandrum]

Great! Then the first step is to clone the RDKit github repo and do a build from source. There are instructions for this (hopefully useable instructions) in the RDKit docs. Once that's all working and you can use the build that you've done inside of a local python environment you're ready to make the required changes.

[greglandrum]

Nadine did some documentation on how we use git/github here: https://github.com/rdkit/UGM_2016/blob/master/Presentations/Landrum_Schneider_GitHub_Git_and_RDKit.pdf
that's still pretty up to date.

[greglandrum]

I will try to provide some pointers on specific code changes tomorrow morning.

[MaxGreil]

@greglandrum Thank you for your advice.
I got everything and the tests running correctly.

[greglandrum]

@MaxGreil : hmm, apologies that I seem to have lost track of this.
Are you still planning to work on it and waiting for me?

[MaxGreil]

Hi @greglandrum,
I am still planning to work on it. Can you please provide some pointers on specific code changes that need o be done?

[greglandrum]

The class MolChemicalFeature has two different getPos() methods:
https://github.com/rdkit/rdkit/blob/master/Code/GraphMol/MolChemicalFeatures/MolChemicalFeature.h#L51
The first takes no arguments and returns the value for the default conformer for that feature.
The second takes confId argument and returns the value for that confId. If confId is -1, it returns the value for the default conformer of the associated molecule.

The Python wrapper for MolChemicalFeature only has one getPos() method:
https://github.com/rdkit/rdkit/blob/master/Code/GraphMol/MolChemicalFeatures/Wrap/MolChemicalFeature.cpp#L54
For this one the confId defaults to -1
So if you call MolChemicalFeature.GetPos() from Python, it ends up calling MolChemicalFeature::getPos(-1) on the C++ side.

the fix for this issue is to add a second method to the python wrapper (in the Wrap/MolChemicalFeature.cpp file) that takes no arguments and that points to the overload MolChemicalFeature::getPos()
You'll need to remove the default value from the method defined on line 54 too (just remove the =-1 bit).

Once you've got that done, it would be good to add a test to Code/GraphMol/MolChemicalFeatures/Wrap/testChemicalFeatures.py that tests the new feature and shows that it works.