search

djay0529 / mdanalysis

Automatically exported from code.google.com/p/mdanalysis
0 stars 0 forks source link

Changing topology reading to be class based #210

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Topology readers are currently just a parse function inside a module.  This 
will get moved to a class based system similar to the trajectory reader system,

Original issue reported on code.google.com by richardjgowers on 4 Feb 2015 at 10:44

GoogleCodeExporter commented 9 years ago

Original comment by richardjgowers on 4 Feb 2015 at 10:44

GoogleCodeExporter commented 9 years ago 
https://code.google.com/p/mdanalysis/source/detail?r=92b11b7264f40022608f15ccc81
97ee7cdee80df&name=feature-classbasedtopologies

Just pushed a work in progress branch for this.

Currently the PDB parsing doesn't work, the segids end up as ' ' and I'm not 
entirely sure why....  The various PDB parsing options are pretty confusing to 
me.

Original comment by richardjgowers on 4 Feb 2015 at 11:04

GoogleCodeExporter commented 9 years ago 
Just did a code review. 

1) Overall: great! Some requests for changes in review.

2) It wasn't clear to me which PDB parser fails. Can you elaborate?

And just to clarify: it also means that this commit breaks PDB reading? (So I 
shouldn't recommend pulling develop at the moment...) – if we can't fix this 
in a day, we should move the cleanup to a feature branch and the revert develop 
to have a working develop.

Original comment by orbeckst on 5 Feb 2015 at 12:40

GoogleCodeExporter commented 9 years ago 
Sorry, failed to mention, it's a feature branch currently, just wanted to make 
sure I'd gone in the right direction.

Thanks for the feedback, I'll keep working on it.  I think differentiating 
between IOError and ValueError make sense so I'll put them back in.

The PDB not working is:

In [1]: import MDAnalysis as mda

In [2]: u = mda.Universe('adk_oplsaa.pdb')

In [3]: u.segments
Out[3]: <SegmentGroup [<Segment ' '>]>

In [4]: len(u.atoms)
Out[4]: 47681

In [5]: len(u.residues)
Out[5]: 11302

In [6]: u.atoms[0]
Out[6]: < Atom 1: name 'N' of type 'N' of resname 'MET', resid 1 and segid ' '>

So for some reason the Segid isn't getting set to "SYSTEM" by default, but 
everything else is fine.  I'll get to the bottom of it.

Original comment by richardjgowers on 5 Feb 2015 at 9:41

GoogleCodeExporter commented 9 years ago

Original comment by richardjgowers on 5 Feb 2015 at 4:31

GoogleCodeExporter commented 9 years ago 
Ok, I think I've finished this now.  Everything relating this passes, (PDB 
reading fixed), so pending review it can be merged to develop.

https://code.google.com/p/mdanalysis/source/detail?r=d07684c9971be1e23c1e4b8ca87
b9c59dd1c1dfc&name=feature-classbasedtopologies

Original comment by richardjgowers on 9 Feb 2015 at 1:18

GoogleCodeExporter commented 9 years ago 
This issue was closed by revision c8bde94ab285.

Original comment by richardjgowers on 9 Feb 2015 at 4:26

GoogleCodeExporter commented 9 years ago 
This issue was closed by revision f3b254426f98.

Original comment by richardjgowers on 16 Feb 2015 at 11:37