From 23bfc48697e3ece121c3e9504eff7500892c9a43 Mon Sep 17 00:00:00 2001 From: IAlibay Date: Sun, 21 Mar 2021 13:00:18 +0000 Subject: [PATCH 1/5] Adds upcoming scale_factor warning --- .../MDAnalysisTests/coordinates/test_netcdf.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py index 81409b82d00..31add954425 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): + with NCDFWriter(outfile) as w: + u = mda.Universe(GRO) + wmsg = "`scale_factor` writing will change" + with pytest.warn(FutureWarning, match=wmsg): + w.write(u.atoms) From 3ca1c57fef7cd6f9fa7dca7cfde1516868fb82b0 Mon Sep 17 00:00:00 2001 From: IAlibay Date: Sun, 21 Mar 2021 13:00:37 +0000 Subject: [PATCH 2/5] Adds scale_factor warning --- package/MDAnalysis/coordinates/TRJ.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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. From 5d83a6a4d8b310b7d6c09d139dd4c8b840a9685d Mon Sep 17 00:00:00 2001 From: IAlibay Date: Sun, 21 Mar 2021 13:06:21 +0000 Subject: [PATCH 3/5] fix test --- testsuite/MDAnalysisTests/coordinates/test_netcdf.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py index 31add954425..7ca88db7915 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py +++ b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py @@ -900,8 +900,8 @@ def test_wrong_n_atoms(self, outfile): w.write(u.trajectory.ts) def test_scale_factor_future(self, outfile): - with NCDFWriter(outfile) as w: - u = mda.Universe(GRO) + u = mda.Universe(GRO) + with NCDFWriter(outfile, u.trajectory.n_atoms) as w: wmsg = "`scale_factor` writing will change" - with pytest.warn(FutureWarning, match=wmsg): + with pytest.warns(FutureWarning, match=wmsg): w.write(u.atoms) From 4bc55f1330f0d4af1877f3d94e0a2771a7e999e7 Mon Sep 17 00:00:00 2001 From: IAlibay Date: Sun, 21 Mar 2021 13:07:20 +0000 Subject: [PATCH 4/5] update changelog --- package/CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package/CHANGELOG b/package/CHANGELOG index c3d2f6a43e9..c1a8e12b6f7 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 tempfactors and bfactors being separate TopologyAttrs, with a warning (PR #3161) * hbonds.WaterBridgeAnalysis will be removed in 2.0.0 and From 915e35426104f38736d7d67d6ed68e427524a8c8 Mon Sep 17 00:00:00 2001 From: IAlibay Date: Sun, 21 Mar 2021 13:31:09 +0000 Subject: [PATCH 5/5] Reverse order, warning is on __init__ --- testsuite/MDAnalysisTests/coordinates/test_netcdf.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py index 7ca88db7915..c8e6544c559 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py +++ b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py @@ -901,7 +901,7 @@ def test_wrong_n_atoms(self, outfile): def test_scale_factor_future(self, outfile): u = mda.Universe(GRO) - with NCDFWriter(outfile, u.trajectory.n_atoms) as w: - wmsg = "`scale_factor` writing will change" - with pytest.warns(FutureWarning, match=wmsg): + 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)