From 04e8af4786a82a0cab388b552d2df3051716bbc0 Mon Sep 17 00:00:00 2001 From: Irfan Alibay Date: Sat, 27 Mar 2021 17:18:53 +0000 Subject: [PATCH] Issue 2327 - Warn of upcoming scale_factor changes (#3185) Towards #2327 # Work done in this PR * Adds warning for upcoming changes to scale_factor writing in NCDFWriter --- package/CHANGELOG | 2 ++ package/MDAnalysis/coordinates/TRJ.py | 19 +++++++++++++++++++ .../coordinates/test_netcdf.py | 15 +++++++++------ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 298ab1c6962..aca247fe2e5 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -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 diff --git a/package/MDAnalysis/coordinates/TRJ.py b/package/MDAnalysis/coordinates/TRJ.py index d87a4ca1824..8e9621ab9f3 100644 --- a/package/MDAnalysis/coordinates/TRJ.py +++ b/package/MDAnalysis/coordinates/TRJ.py @@ -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` @@ -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. diff --git a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py index 81409b82d00..c8e6544c559 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py +++ b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py @@ -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, @@ -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)