MDAnalysis / mdanalysis

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

API should be consistent? #277

Closed hainm closed 9 years ago

hainm commented 9 years ago

hi guys,

this is another attempt to make a connection between pytraj and MDAnalysis. I still have really hard time to play with MDA, mostly because the API not really consistent (in my view).

I was trying to convert Universe object to pytraj Topology. what I am stuck here is I can not take the bond indices from Universe when loading Amber prmtop file. This is OK with PSF + DCD file. It's clearly that MDAnalysis gives empty bond indices for Amber top and non-empty one for PSF.

https://github.com/hainm/pytraj/blob/load_from_mdanalysis/tests/test_load_MDAnalysis_inconsistent.py#L10

My solution is to load Amber Topology separately from pytraj and load only coords from MDanalysis. But I think this is not the best solution if I want to load other file formats that MDA understands but pytraj does not.

So my question is: Am I missing something? Is there any inconsistent in API I need to know about?

Hai

hainm commented 9 years ago

ah, this is my draft for creating TrajectoryMDanalysisIterator in pytraj

https://github.com/hainm/pytraj/blob/load_from_mdanalysis/pytraj/trajs/TrajectoryMDAnalysisIterator.py

test: https://github.com/hainm/pytraj/blob/load_from_mdanalysis/tests/test_MDAnalysisIterator.py

hainm commented 9 years ago

it's likely that I should use try an except (a lot) to load. https://github.com/pytraj/pytraj/commit/d928d823f7a74b4fe1f249d8a30a6681b48a318d

richardjgowers commented 9 years ago

Hi

The Prmtop reader doesn't read bonds currently.

Richard

orbeckst commented 9 years ago

@hainm , please open an issue for the PRMTOP reader if you need it to read bonds. Thanks!

hainm commented 9 years ago

@orbeckst i am more interested in another formats (since pytraj can easily read prmtop). does mda read bonds for other formats? if not, I can open another issue.

hai

orbeckst commented 9 years ago

On 19 May, 2015, at 11:52, Hai Nguyen wrote:

@orbeckst i am more interested in another formats (since pytraj can easily read prmtop). does mda read bonds for other formats? if not, I can open another issue.

We can guess bonds and in principle read them from PSF, TPR. MOL2, PDB (IIRC) � but @richardgowers might know more about the specifics and there's at least one open issue regarding bonds in the TPR reader #222.

Generally: raise issues for anything that you care about. As you know, with open source projects, people's time is a precious resource and not everything will get done as soon as you'd like/need it to but at least the problems won't be forgotten and the issues also work as a tool to prioritize. Of course, we're also happy to consider pull requests.

richardjgowers commented 9 years ago

I think that about covers it, there's

u = mda.Universe(..........., guess_bonds=True [, vdwradii={atomtype: vdwradius}])

Which can guess bonds as a last resort.

I think there's not really a nice way to know you're missing bonds and raise an error. Eg how can we differentiate between a box of argon and a box with no bonds? Which then leads to it being a bit confusing, with the Universe saying "there's no bonds" rather than realising "there are bonds but I don't know them".

hainm commented 9 years ago

@richardjgowers I got empty tuple () when calling bonds.to_indices(). is this a way MDA saying "there is no bonds". just want to ask to make sure.

richardjgowers commented 9 years ago

Yeah I think that's just designed to dump the contents, so an object with 0 size giving an output of 0 length made sense to me at the time.

The bonds should evaluate to False if they're empty, so

if u.bonds:
    # do stuff with bonds
else:
    # there are no bonds

Might work for you?

hainm commented 9 years ago

yes I think so. thank you. (I always forgot empty is False in Python)

Hai

On Tuesday, May 19, 2015, Richard Gowers notifications@github.com wrote:

Yeah I think that's just designed to dump the contents, so an object with 0 size giving an output of 0 length made sense to me at the time.

The bonds should evaluate to False if they're empty, so

if u.bonds:

do stuff with bondselse:

# there are no bonds

Might work for you?

— Reply to this email directly or view it on GitHub https://github.com/MDAnalysis/mdanalysis/issues/277#issuecomment-103646487 .

Hai Nguyen