[Biopython-dev] [biopython] Added transform and copy method to Entity in biopython.PDB (#25)

[Eric Talevich]

On Tue, Feb 7, 2012 at 10:36 AM, Eric Talevich <eric.talevich at gmail.com>wrote:

> On Tue, Feb 7, 2012 at 8:46 AM, Peter Cock <p.j.a.cock at googlemail.com>wrote:
>
>> Sorry, didn't realise this was direct to me and not the mailing list.
>> Would everyone be OK with git pull requests coming straight here?
>>
>> Anyway - Eric, would this be something you could look at?
>>
>> Peter
>>
>
> At a glance, the code looks cool to me, though I faintly recall some
> overlap with João's unmerged work (copy method). I'll try to find time to
> test it, but would not be upset if someone else got to it first.
> -E
>
>
(Not sure who gets follow-up e-mails from Github, if anyone.)

Could someone else confirm whether the last patch is correct? It switches
the order of the dot product arguments in Atom.transform().
https://github.com/benreynwar/biopython/commit/346df8f2006735129a93508a04c4cdf6acb99a5f

Code:


    def transform(self, rot, tran):
        """
        Apply rotation and translation to the atomic coordinates.

        Example:
                >>> rotation=rotmat(pi, Vector(1,0,0))
                >>> translation=array((0,0,1), 'f')
                >>> atom.transform(rotation, translation)

        @param rot: A right multiplying rotation matrix
        @type rot: 3x3 Numeric array

        @param tran: the translation vector
        @type tran: size 3 Numeric array
        """

-        self.coord=numpy.dot(self.coord, rot)+tran
+        self.coord=numpy.dot(rot, self.coord)+tran


This will break every script that uses the transform() method if we apply
it. It also breaks the unit test, of course, but I can change the unit test
to match if we accept this patch.

It seems to me that which way is right is a matter of how the user
specifies the input. I'm not a thinking man, so I don't entirely trust my
judgment on this one.

Thanks,
Eric



>
>> ---------- Forwarded message ----------
>> From: benreynwar
>> <
>> reply+i-2958669-14408e039dee774169d6f09c683146c3f42dd0b9-63959 at reply.github.com
>> >
>> Date: Tue, Jan 24, 2012 at 11:12 PM
>> Subject: [biopython] Added transform and copy method to Entity in
>> biopython.PDB (#25)
>> To: Peter Cock <p.j.a.cock at googlemail.com>
>>
>>
>> Minor changes to biopython.PDB to allow transforms to be applied to
>> entities and copies of entities to be made.  Also fixed a bug in the
>> transform method of Atom.  Tests are included.
>> The changes are from a few months ago but they still merge cleanly
>> into the current master.
>>
>> You can merge this Pull Request by running:
>>
>>  git pull https://github.com/benreynwar/biopython master
>>
>> Or you can view, comment on it, or merge it online at:
>>
>>  https://github.com/biopython/biopython/pull/25
>>
>> -- Commit Summary --
>>
>> * Added tranform method to Entity.
>> * Add test for transform method.
>> * Adding copy method for Entities
>> * Added an insert method to Entity.  This allows a child to be
>> inserted into a specified position in the child_list which effects
>> position in the PDB output.
>> * Fixed bug in transform method of Atom (dot product order).
>>
>> -- File Changes --
>>
>> M Bio/PDB/Atom.py (14)
>> M Bio/PDB/Entity.py (40)
>> M Tests/test_PDB.py (70)
>>
>> -- Patch Links --
>>
>>  https://github.com/biopython/biopython/pull/25.patch
>>  https://github.com/biopython/biopython/pull/25.diff
>>
>> ---
>> Reply to this email directly or view it on GitHub:
>> https://github.com/biopython/biopython/pull/25
>>
>
>