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

Fixes issues 3166 and 3255 #3256

Merged
merged 4 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ The rules for this file:
* 2.0.0

Fixes
* NCDFReader now defaults to a dt value of 1.0 ps when it cannot estimate
it from the first two frames of the file (Issue #3166)
* Get chi1_selection to match canonical gamma atoms (Issue #2044)
* ITPParser now accepts relative paths (Issue #3037, PR #3108)
* Fixed issue with unassigned 'GAP' variable in fasta2algin function when
Expand Down Expand Up @@ -211,6 +213,9 @@ Changes
* `threadpoolctl` is required for installation (Issue #2860)

Deprecations
* In 2.1.0 the TRZReader will default to a dt of 1.0 ps when failing to
read it from the input TRZ trajectory.


06/09/20 richardjgowers, kain88-de, lilyminium, p-j-smith, bdice, joaomcteixeira,
PicoCentauri, davidercruz, jbarnoud, RMeli, IAlibay, mtiberti, CCook96,
Expand Down
Empty file modified package/MDAnalysis/coordinates/NAMDBIN.py
100755 → 100644
Empty file.
13 changes: 11 additions & 2 deletions package/MDAnalysis/coordinates/TRJ.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ class NCDFReader(base.ReaderBase):
.. versionchanged:: 2.0.0
Now use a picklable :class:`scipy.io.netcdf.netcdf_file`--
:class:`NCDFPicklable`.
Reading of `dt` now defaults to 1.0 ps if `dt` cannot be extracted from
the first two frames of the trajectory.

"""

Expand Down Expand Up @@ -694,8 +696,15 @@ def _read_next_timestep(self, ts=None):
raise IOError from None

def _get_dt(self):
t1 = self.trjfile.variables['time'][1]
t0 = self.trjfile.variables['time'][0]
"""Gets dt based on the time difference between the first and second
frame. If missing (i.e. an IndexError is triggered), raises an
AttributeError which triggers the default 1 ps return of dt().
"""
try:
t1 = self.trjfile.variables['time'][1]
t0 = self.trjfile.variables['time'][0]
except IndexError:
raise AttributeError
return t1 - t0

def close(self):
Expand Down
3 changes: 3 additions & 0 deletions package/MDAnalysis/coordinates/TRZ.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ def _get_dt(self):
t1 = self.ts.time
dt = t1 - t0
except StopIteration:
msg = ('dt information could not be obtained, defaulting to 0 ps. '
'Note: in MDAnalysis 2.1.0 this default will change 1 ps.')
warnings.warn(msg)
return 0
Copy link
Member Author

Choose a reason for hiding this comment

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

This is inconsistent with what we do in base.py, ideally this should just throw an AttributeError and default on what's done in base?

else:
return dt
Expand Down
19 changes: 14 additions & 5 deletions testsuite/MDAnalysisTests/coordinates/test_netcdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,19 @@ def test_program_warn(self, tmpdir, mutation):
"attributes are missing")
assert str(record[0].message.args[0]) == wmsg

def test_no_dt_warning(self, tmpdir):
"""Issue 3166 - not being able to call dt should throw a warning.
Also at the same time checks that single frame chain reading works"""
u = mda.Universe(PFncdf_Top, PFncdf_Trj)
with tmpdir.as_cwd():
with NCDFWriter('single_frame.nc', u.atoms.n_atoms) as W:
W.write(u)

# Using the ChainReader implicitly calls dt() and thus _get_dt()
wmsg = "Reader has no dt information, set to 1.0 ps"
with pytest.warns(UserWarning, match=wmsg):
u2 = mda.Universe(PFncdf_Top, [PFncdf_Trj, 'single_frame.nc'])


class _NCDFWriterTest(object):
prec = 5
Expand Down Expand Up @@ -891,20 +904,16 @@ def test_writer_units(self, outfile, var, expected):
assert_equal(unit, expected)


class TestNCDFWriterErrors(object):
class TestNCDFWriterErrorsWarnings(object):
@pytest.fixture()
def outfile(self, tmpdir):
return str(tmpdir) + 'out.ncdf'

def test_zero_atoms_VE(self, outfile):
from MDAnalysis.coordinates.TRJ import NCDFWriter

with pytest.raises(ValueError):
NCDFWriter(outfile, 0)

def test_wrong_n_atoms(self, outfile):
from MDAnalysis.coordinates.TRJ import NCDFWriter

with NCDFWriter(outfile, 100) as w:
u = make_Universe(trajectory=True)
with pytest.raises(IOError):
Expand Down
24 changes: 18 additions & 6 deletions testsuite/MDAnalysisTests/coordinates/test_trz.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,28 @@ class TestTRZWriter2(object):
def u(self):
return mda.Universe(two_water_gro)

def test_writer_trz_from_other(self, u, tmpdir):
outfile = os.path.join(str(tmpdir), 'trz-writer-2.trz')
@pytest.fixture()
def outfile(self, tmpdir):
return str(tmpdir.join('/trz-writer-2.trz'))

def test_writer_trz_from_other(self, u, outfile):
with mda.coordinates.TRZ.TRZWriter(outfile, len(u.atoms)) as W:
W.write(u)

u2 = mda.Universe(two_water_gro, outfile)

assert_almost_equal(u.atoms.positions, u2.atoms.positions, 3)

def test_no_dt_warning(self, u, outfile):
with mda.coordinates.TRZ.TRZWriter(outfile, len(u.atoms)) as W:
W.write(u)
W.close()

u2 = mda.Universe(two_water_gro, outfile)
u2 = mda.Universe(two_water_gro, outfile)

assert_almost_equal(u.atoms.positions,
u2.atoms.positions, 3)
wmsg = ('dt information could not be obtained, defaulting to 0 ps. '
'Note: in MDAnalysis 2.1.0 this default will change 1 ps.')
with pytest.warns(UserWarning, match=wmsg):
assert_almost_equal(u2.trajectory.dt, 0)


class TestWrite_Partial_Timestep(object):
Expand Down