Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated to return Permissive PDB Reader and Parser for all PDBs #812

Closed
wants to merge 10 commits into from
Prev Previous commit
Next Next commit
Changed tests to eliminate known failures
caused by BioPython, updated CHANGELOG.
  • Loading branch information
jdetle committed Apr 6, 2016
commit 2ee04ba3965230c1b310b9b1cc8acf74be30a254
15 changes: 9 additions & 6 deletions package/MDAnalysis/coordinates/PDB.py
Original file line number Diff line number Diff line change
Expand Up @@ -1055,16 +1055,19 @@ def CONECT(self, conect):
conect = "".join(conect)
self.pdbfile.write(self.fmt['CONECT'].format(conect))

warnings.warn('PrimitivePDBReader is identical to the PDBReader,'
'it is deprecated in favor of the shorter name',
category=DeprecationWarning)

class PrimitivePDBReader(PDBReader):
warnings.warn('PrimitivePDBReader is identical to the PDBReader,'
'it is deprecated in favor of the shorter name'
'removal targeted for version 0.15.0',
category=DeprecationWarning)
format = 'Permissive_PDB'

warnings.warn('PrimitivePDBWriter is now identical to the PDBWriter,'
'it is deprecated in favor of the shorter name',
category=DeprecationWarning)
class PrimitivePDBWriter(PDBWriter):
warnings.warn('PrimitivePDBWriter is identical to the Writer,'
'it is deprecated in favor of the shorter name'
'removal targeted for version 0.15.0',
category=DeprecationWarning)
format = 'Permissive_PDB'

class ExtendedPDBReader(PDBReader):
Expand Down
15 changes: 8 additions & 7 deletions package/MDAnalysis/topology/PrimitivePDBParser.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,18 @@
from .base import TopologyReader


warnings.warn('PrimitivePDBParser is identical to the PDBParser,'
'it is deprecated in favor of the shorter name',
category=DeprecationWarning)

class PrimitivePDBParser(PDBParser.PDBParser):
Copy link
Member

@kain88-de kain88-de Apr 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you rename this whole file to PDBParser and remove the old Bio Python PDB Topology parser?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's exactly what I did. This is just a placeholder in the same way that PrimitivePDBReader PrimitivePDBWriter is now a placeholder.

warnings.warn('PrimitivePDBParser is identical to the PDBParser,'
'it is deprecated in favor of the shorter name',
category=DeprecationWarning)
format = 'Permissive_PDB'

warnings.warn('PrimitivePDBParser is identical to the PDBParser,'
'it is deprecated in favor of the shorter name,'
'please use the parse_conect function of that class',
category=DeprecationWarning)
def _parse_conect(conect):
warnings.warn('PrimitivePDBParser module is identical to the PDBParser,'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oO why is this warning here in a private method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops.

'module, it is deprecated in favor of the shorter name,'
'please use the parse_conect function of that class',
category=DeprecationWarning)
"""parse a CONECT record from pdbs

Parameters
Expand Down
4 changes: 2 additions & 2 deletions testsuite/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ Also see https://github.com/MDAnalysis/mdanalysis/wiki/MDAnalysisTests
and https://github.com/MDAnalysis/mdanalysis/wiki/UnitTests

------------------------------------------------------------------------------
??/??/16 orbeckst, jbarnoud, pedrishi, fiona-naughton
??/??/16 jdetle, orbeckst, jbarnoud, pedrishi, fiona-naughton
* 0.15.0

- removed known failures from BioPython
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed biopython PDB parser for coordinates and topology (Issue #number)

- metadata update: link download_url to GitHub releases so that
Depsy recognizes contributors (issue #749) and added
@richardjgowers as maintainer
Expand Down
15 changes: 8 additions & 7 deletions testsuite/MDAnalysisTests/coordinates/test_pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,19 @@ def setUp(self):
# http://www.wwpdb.org/documentation/format32/sect9.html#ATOM
self.prec = 3

def test_uses_Biopython(self):

def test_uses_PDBReader(self):
from MDAnalysis.coordinates.PDB import PDBReader

assert_(isinstance(self.universe.trajectory, PDBReader),
"failed to choose Biopython PDBReader")
"failed to choose PDBReader")


@knownfailure("Biopython PDB reader does not parse CRYST1", AssertionError)
def test_dimensions(self):
assert_almost_equal(
self.universe.trajectory.ts.dimensions, RefAdKSmall.ref_unitcell,
self.prec,
"Biopython reader failed to get unitcell dimensions from CRYST1")
"PDBReader failed to get unitcell dimensions from CRYST1")


class _PDBMetadata(TestCase, Ref4e43):
Expand Down Expand Up @@ -247,7 +248,7 @@ def test_check_coordinate_limits_max(self):
def test_check_header_title_multiframe(self):
"""Check whether HEADER and TITLE are written just once in a multi-
frame PDB file (Issue 741)"""
u = mda.Universe(PSF,DCD, permissive=True)
u = mda.Universe(PSF,DCD, permissive=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permissive should be removed. This has to work with the normal PDB parser now.

pdb = mda.Writer(self.outfile, multiframe=True)
protein = u.select_atoms("protein and name CA")
for ts in u.trajectory[:5]:
Expand Down Expand Up @@ -713,11 +714,11 @@ def setUp(self):
# http://www.wwpdb.org/documentation/format32/sect9.html#ATOM
self.prec = 3

def test_uses_Biopython(self):
def test_uses_PDBReader(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check that we don't have the same test setup with the old primitivePDBReader/Writer. If that is the case you can delete this test completely.

from MDAnalysis.coordinates.PDB import PDBReader

assert_(isinstance(self.universe.trajectory, PDBReader),
"failed to choose Biopython PDBReader")
"failed to choose PDBReader")


class TestPDBWriterOccupancies(object):
Expand Down
2 changes: 1 addition & 1 deletion testsuite/MDAnalysisTests/test_streamio.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def test_PrimitivePDBReader(self):
u = MDAnalysis.Universe(streamData.as_NamedStream('PDB'))
assert_equal(u.atoms.n_atoms, self.ref_n_atoms)

@knownfailure()

def test_PDBReader(self):
try:
u = MDAnalysis.Universe(streamData.as_NamedStream('PDB'), permissive=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the permissive flag should be removed here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also why does this pass now?

Expand Down