MDAnalysis / mdanalysis

MDAnalysis is a Python library to analyze molecular dynamics simulations.
https://mdanalysis.org
Other
1.32k stars 652 forks source link

add __iter__ for Universe? #272

Closed hainm closed 9 years ago

hainm commented 9 years ago

I am not sure your philosophy in designing mdanalysis, but my first though when creating something like traj (from pytraj, mdtraj, ...) or Universe in mdanalysis is to try to iterate it.

for my_attempt in Universe(PSF, DCD):
    pass

I think it's great if several packages share similar behavior (somewhat) so user can jump from one to another without too much learning.

What do you guys think? Hai

richardjgowers commented 9 years ago

I think Universe is designed to be the central point for everything about the MD data, so it also has .atoms .residues etc... Which means the iteration is possibly ambiguous.

I also think it reads pretty well, "for timestep in trajectory:"

for timestep in u.trajectory:
    # do analysis

All that said, I do agree that iterating over traj is the most common operation, so it would be very convenient

Not sure..

mnmelo commented 9 years ago

I'm not sure the objective here should be to clone behavior just for the sake of interpackage consistency.

In any case, in order to preserve both approaches we could define _iter_/_getitem_ etc. based on a flag passed to the Universe initializer:

u = MDAnalysis.Universe("sometopology", "sometrajectory", expose_trajectory=True)

If this is still not compatible enough (because now there's the need for a flag whereas other packages do it by default), we could subclass Universe (say, as UniverseTraj) with those same _iter_/_getitem_ etc. Users interested in interpackage compatibility need only instantiate UniverseTraj instead of Universe.

hainm commented 9 years ago

I not saying about clone. I am saying about common interface. (of course, you can enjoy staying out of python ecosystem if you want). my 5 cents.

Hai

mnmelo commented 9 years ago

Don't get me wrong, I certainly see the place for such interchangeability. The solutions I gave would let everyone have their cake and eat each other's too.

We might even envision a Universe subclass with further adapted names to quack like a pytraj or mdtraj object. I'm sure such an effort will find aspects of the APIs that are irreconcilable, but might still be a worthy approach.

I simply feel that these interchangeability considerations, desirable as they might be, should not rank so high that they constrain the development of MDAnalysis' API.

orbeckst commented 9 years ago

I find an iterator on Universe too ambiguous to be useful, and you could equally argue that pytraj and mdtraj should adopt the more explicit MDAnalysis convention to iterate over a trajectory object (see the Python Zen...).

In fact the MDAnalysis philosophy does differ from, say, mdtraj (as far as I understand it), in that the topology is central (because it is unchanging) whereas coordinates are adding additional information to the atoms. In that sense it is more similar to, say, LOOS and of course MMTK (to which all of these tools owe some historic debt).

So, in summary I don't see a big advantage for changing our API in this direction. If a user (such as @hainm ) feels very strongly about this, it's not hard to derive a class such as

import MDAnalysis
class MyUniverse(MDAnalysis.Universe):
   def __iter__(self):
         return self.trajectory
hainm commented 9 years ago

hi,

now I see the philosophy of designing mdanalysis now.

i already wrote wrapper here https://github.com/pytraj/pytraj/blob/master/pytraj/trajs/TrajectoryMDAnalysisIterator.py

cheers Hai