Transform3D#SetRotation() around arbitrary axis scales coordinates

[sroughley]

The method Transform3D#SetRotation(double, Point3D) scales the co-ordinates if the axis vector Point3D is not a unit vector:

rdkit/Code/Geometry/Transform3D.h

Lines 64 to 69 in 7e575db

	/*! \brief set the rotation matrix
	*
	* The rotation matrix is set to rotation by th specified angle
	* about an arbitrary axis
	*/
	void SetRotation(double angle, const Point3D &axis);

Either the documentation should state this requirement, or the method implement it.

This can be demonstrated in the Java wrapper as follows:

String molBlock = "Single Atom should be invariate\n     RDKit          3D\n\n  1  0  0  0  0  0  0  0  0  0999 V2000\n"+
"    1.0000    1.0000    1.0000 C   0  0  0  0  0  0  0  0  0  0  0  0\nM  END\n\n$$$$";
ROMol mol = RWMol.MolFromMolBlock(molBlock);

// An axis around which the molecule should be invariate:
Point3D axis = new Point3D(1.0, 1.0, 1.0);

//The transform
Transform3D t3d = new Transform3D();
t3d.SetRotation(Math.PI, axis);

//Apply it...
mol.transformMolsAtoms(t3d);

System.out.println(mol.MolToMolBlock());

Gives:

Single Atom should be invariate
     RDKit          3D

  1  0  0  0  0  0  0  0  0  0999 V2000
    3.0000    3.0000    3.0000 C   0  0  0  0  0  0  0  0  0  0  0  0
M  END

The result can be correct in the calling code, but this is not obviously required from the documentation, e.g.:

...
// An axis around which the molecule should be invariate:
Point3D axis = new Point3D(1.0, 1.0, 1.0);
axis.normalize();
...

[greglandrum]

I agree that the behavior should be properly documented.
There's an argument for changing it, but that would remove functionality and potentially break a lot of existing code.

[sroughley]

Hi @greglandrum - I think we are agreed - the behaviour change would be 'ideal' but the breakages could be... ouch.

I will add a sentence to the doc comment in the header file and submit a pull request if you think that would cover it?

[greglandrum]

@sroughley

I will add a sentence to the doc comment in the header file and submit a pull request if you think that would cover it?

That would be perfect. Thanks!