Skip to content

Commit

Permalink
Issue 2327 - Warn of upcoming scale_factor changes (#3185)
Browse files Browse the repository at this point in the history
Towards #2327 

# Work done in this PR
* Adds warning for upcoming changes to scale_factor writing in NCDFWriter
  • Loading branch information
IAlibay authored Mar 27, 2021
1 parent 9a03aec commit 04e8af4
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
2 changes: 2 additions & 0 deletions package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ Changes
* Maximum pinned versions in setup.py removed for python 3.6+ (PR #3139)

Deprecations
* NCDFWriter `scale_factor` writing will change in version 2.0 to
better match AMBER outputs (Issue #2327)
* Deprecated using the last letter of the segid as the
chainID when writing PDB files (Issue #3144)
* Deprecated tempfactors and bfactors being separate
Expand Down
19 changes: 19 additions & 0 deletions package/MDAnalysis/coordinates/TRJ.py
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,18 @@ class NCDFWriter(base.WriterBase):
.. _`Issue #506`:
https://github.com/MDAnalysis/mdanalysis/issues/506#issuecomment-225081416
Raises
------
FutureWarning
When writing. The :class:`NCDFWriter` currently does not write any
`scale_factor` values for the data variables. Whilst not in breach
of the AMBER NetCDF standard, this behaviour differs from that of
most AMBER writers, especially for velocities which usually have a
`scale_factor` of 20.455. In MDAnalysis 2.0, the :class:`NCDFWriter`
will enforce `scale_factor` writing to either match user inputs (either
manually defined or via the :class:`NCDFReader`) or those as written by
AmberTools's :program:`sander` under default operation.
See Also
--------
:class:`NCDFReader`
Expand Down Expand Up @@ -876,6 +888,13 @@ def __init__(self,
self.has_forces = kwargs.get('forces', False)
self.curr_frame = 0

# warn users of upcoming changes
wmsg = ("In MDAnalysis v2.0, `scale_factor` writing will change ("
"currently these are not written), to better match the way "
"they are written by AMBER programs. See NCDFWriter docstring "
"for more details.")
warnings.warn(wmsg, FutureWarning)

def _init_netcdf(self, periodic=True):
"""Initialize netcdf AMBER 1.0 trajectory.
Expand Down
15 changes: 9 additions & 6 deletions testsuite/MDAnalysisTests/coordinates/test_netcdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
assert_equal,
assert_almost_equal
)
from MDAnalysis.coordinates.TRJ import NCDFReader
from MDAnalysis.coordinates.TRJ import NCDFReader, NCDFWriter

from MDAnalysisTests.datafiles import (PFncdf_Top, PFncdf_Trj,
GRO, TRR, XYZ_mini,
Expand Down Expand Up @@ -884,21 +884,24 @@ 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):
w.write(u.trajectory.ts)

def test_scale_factor_future(self, outfile):
u = mda.Universe(GRO)
wmsg = "`scale_factor` writing will change"
with pytest.warns(FutureWarning, match=wmsg):
with NCDFWriter(outfile, u.trajectory.n_atoms) as w:
w.write(u.atoms)

0 comments on commit 04e8af4

Please sign in to comment.