diff --git a/package/AUTHORS b/package/AUTHORS index 35e4029f77..5c75da76af 100644 --- a/package/AUTHORS +++ b/package/AUTHORS @@ -236,6 +236,7 @@ Chronological list of authors 2024 - Aditya Keshari - Philipp Stärk + - Valerij Talagayev External code ------------- diff --git a/package/CHANGELOG b/package/CHANGELOG index 094ec0091c..81bcbac86a 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -16,7 +16,7 @@ The rules for this file: ------------------------------------------------------------------------------- ??/??/?? IAlibay, HeetVekariya, marinegor, lilyminium, RMeli, ljwoods2, aditya292002, pstaerk, PicoCentauri, BFedder, - tyler.je.reddy + tyler.je.reddy, talagayev * 2.8.0 @@ -34,6 +34,7 @@ Fixes * Fix deploy action to use the correct version of the pypi upload action. Enhancements + * Change order of arguments for adding auxiliary data (Issue #3811) * Improved performance of PDBWriter (Issue #2785, PR #4472) * Added parsing of arbitrary columns of the LAMMPS dump parser. (Issue #3504) * Documented the r0 attribute in the `Contacts` class and added the diff --git a/package/MDAnalysis/auxiliary/EDR.py b/package/MDAnalysis/auxiliary/EDR.py index fe9173b752..3d2b3ec9c0 100644 --- a/package/MDAnalysis/auxiliary/EDR.py +++ b/package/MDAnalysis/auxiliary/EDR.py @@ -142,8 +142,10 @@ using this method. The data is then accessible in the `ts.aux` namespace via both attribute and dictionary syntax:: - In [4]: u.trajectory.add_auxiliary({"epot": "Potential", - "angle": "Angle"}, aux) + In [4]: u.trajectory.add_auxiliary(aux, + {"epot": "Potential", + "angle": "Angle"}, + ) In [5]: u.trajectory.ts.aux.epot Out[5]: -525164.0625 In [6]: u.trajectory.ts.aux.Angle diff --git a/package/MDAnalysis/auxiliary/__init__.py b/package/MDAnalysis/auxiliary/__init__.py index dff7778674..521df120aa 100644 --- a/package/MDAnalysis/auxiliary/__init__.py +++ b/package/MDAnalysis/auxiliary/__init__.py @@ -131,7 +131,7 @@ guess an appropriate reader. e.g.:: u = MDAnalysis.Universe(PDB, XTC) - u.trajectory.add_auxiliary('pullforce', './pull_force.xvg') + u.trajectory.add_auxiliary('./pull_force.xvg', 'pullforce') As the trajectory frame is updated, the auxiliary data will be read correspondingly, @@ -149,7 +149,7 @@ avoid representative values set to ``np.nan``, particularly when auxiliary data is less frequent:: - u.trajectory.add_auxiliary('low_f', 'low_freq_aux_data.xvg') + u.trajectory.add_auxiliary('low_freq_aux_data.xvg', 'low_f') for ts in u.trajectory.iter_as_aux('low_f'): do_something(ts.aux.low_f) # do something with 'low_f' auxiliary data without # worrying about having to deal with np.nan diff --git a/package/MDAnalysis/coordinates/base.py b/package/MDAnalysis/coordinates/base.py index 45befe1c55..06233eef52 100644 --- a/package/MDAnalysis/coordinates/base.py +++ b/package/MDAnalysis/coordinates/base.py @@ -1073,10 +1073,9 @@ def timeseries(self, asel: Optional['AtomGroup']=None, raise ValueError(errmsg) return coordinates -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def add_auxiliary(self, + auxdata: Union[str, AuxReader], aux_spec: Union[str, Dict[str, str]] = None, - auxdata: Union[str, AuxReader] = None, format: str = None, **kwargs) -> None: """Add auxiliary data to be read alongside trajectory. @@ -1101,7 +1100,7 @@ def add_auxiliary(self, pull-force.xvg:: u = MDAnalysis.Universe(PDB, XTC) - u.trajectory.add_auxiliary('pull', 'pull-force.xvg') + u.trajectory.add_auxiliary('pull-force.xvg', 'pull') The representative value for the current timestep may then be accessed as ``u.trajectory.ts.aux.pull`` or ``u.trajectory.ts.aux['pull']``. @@ -1120,7 +1119,7 @@ def add_auxiliary(self, added as identified by a :attr:`data_selector`:: term_dict = {"temp": "Temperature", "epot": "Potential"} - u.trajectory.add_auxiliary(term_dict, "ener.edr") + u.trajectory.add_auxiliary("ener.edr", term_dict) Adding this data can be useful, for example, to filter trajectory frames based on non-coordinate data like the potential energy of each @@ -1140,9 +1139,6 @@ def add_auxiliary(self, Auxiliary data is assumed to be time-ordered, with no duplicates. See the :ref:`Auxiliary API`. """ - if auxdata is None: - raise ValueError("No input `auxdata` specified, but it needs " - "to be provided.") if type(auxdata) not in list(_AUXREADERS.values()): # i.e. if auxdata is a file, not an instance of an AuxReader reader_type = get_auxreader_for(auxdata) @@ -1523,7 +1519,7 @@ def copy(self): # been modified since initial load new.ts = self.ts.copy() for auxname, auxread in self._auxs.items(): - new.add_auxiliary(auxname, auxread.copy()) + new.add_auxiliary(auxread.copy(), auxname) return new def __del__(self): @@ -1701,7 +1697,7 @@ def copy(self): new.ts = self.ts.copy() for auxname, auxread in self._auxs.items(): - new.add_auxiliary(auxname, auxread.copy()) + new.add_auxiliary(auxread.copy(), auxname) # since the transformations have already been applied to the frame # simply copy the property new.transformations = self.transformations diff --git a/package/MDAnalysis/coordinates/memory.py b/package/MDAnalysis/coordinates/memory.py index d2521b9a21..8a5dfab08e 100644 --- a/package/MDAnalysis/coordinates/memory.py +++ b/package/MDAnalysis/coordinates/memory.py @@ -451,7 +451,7 @@ def copy(self): new[self.ts.frame] for auxname, auxread in self._auxs.items(): - new.add_auxiliary(auxname, auxread.copy()) + new.add_auxiliary(auxread.copy(), auxname) # since transformations are already applied to the whole trajectory # simply copy the property new.transformations = self.transformations diff --git a/testsuite/MDAnalysisTests/auxiliary/base.py b/testsuite/MDAnalysisTests/auxiliary/base.py index 2f40044405..4d32717e41 100644 --- a/testsuite/MDAnalysisTests/auxiliary/base.py +++ b/testsuite/MDAnalysisTests/auxiliary/base.py @@ -128,7 +128,7 @@ def format_data(self, data): class BaseAuxReaderTest(object): def test_raise_error_no_auxdata_provided(self, ref, ref_universe): - with pytest.raises(ValueError, match="No input `auxdata`"): + with pytest.raises(TypeError): ref_universe.trajectory.add_auxiliary() def test_n_steps(self, ref, reader): diff --git a/testsuite/MDAnalysisTests/auxiliary/test_edr.py b/testsuite/MDAnalysisTests/auxiliary/test_edr.py index 224abba4ea..9dcbe09ae5 100644 --- a/testsuite/MDAnalysisTests/auxiliary/test_edr.py +++ b/testsuite/MDAnalysisTests/auxiliary/test_edr.py @@ -195,8 +195,7 @@ def ref(): @pytest.fixture def ref_universe(ref): u = mda.Universe(AUX_EDR_TPR, AUX_EDR_XTC) -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 - u.trajectory.add_auxiliary({"test": "Bond"}, ref.testdata) + u.trajectory.add_auxiliary(ref.testdata, {"test": "Bond"}) return u @staticmethod @@ -309,7 +308,6 @@ def test_represent_as_average_with_cutoff(self, ref, reader): err_msg="Representative value does not match when " "applying cutoff") -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_add_all_terms_from_file(self, ref, ref_universe): ref_universe.trajectory.add_auxiliary(auxdata=ref.testdata) # adding "test" manually to match above addition of test term @@ -317,57 +315,54 @@ def test_add_all_terms_from_file(self, ref, ref_universe): terms = [key for key in ref_universe.trajectory._auxs] assert ref_terms == terms -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_add_all_terms_from_reader(self, ref_universe, reader): ref_universe.trajectory.add_auxiliary(auxdata=reader) ref_terms = ["test"] + [key for key in get_auxstep_data(0).keys()] terms = [key for key in ref_universe.trajectory._auxs] assert ref_terms == terms -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_add_term_list_custom_names_from_file(self, ref, ref_universe): - ref_universe.trajectory.add_auxiliary({"bond": "Bond", + ref_universe.trajectory.add_auxiliary(ref.testdata, + {"bond": "Bond", "temp": "Temperature"}, - ref.testdata) + ) ref_dict = get_auxstep_data(0) assert ref_universe.trajectory.ts.aux.bond == ref_dict["Bond"] assert ref_universe.trajectory.ts.aux.temp == ref_dict["Temperature"] -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_add_term_list_custom_names_from_reader(self, ref_universe, reader): - ref_universe.trajectory.add_auxiliary({"bond": "Bond", + ref_universe.trajectory.add_auxiliary(reader, + {"bond": "Bond", "temp": "Temperature"}, - reader) + ) ref_dict = get_auxstep_data(0) assert ref_universe.trajectory.ts.aux.bond == ref_dict["Bond"] assert ref_universe.trajectory.ts.aux.temp == ref_dict["Temperature"] -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_raise_error_if_auxname_already_assigned(self, ref_universe, reader): with pytest.raises(ValueError, match="Auxiliary data with name"): - ref_universe.trajectory.add_auxiliary("test", reader, "Bond") + ref_universe.trajectory.add_auxiliary(reader, "test", "Bond") -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_add_single_term_custom_name_from_file(self, ref, ref_universe): - ref_universe.trajectory.add_auxiliary({"temp": "Temperature"}, - ref.testdata) + ref_universe.trajectory.add_auxiliary(ref.testdata, + {"temp": "Temperature"}, + ) ref_dict = get_auxstep_data(0) assert ref_universe.trajectory.ts.aux.temp == ref_dict["Temperature"] -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_add_single_term_custom_name_from_reader(self, ref_universe, reader): - ref_universe.trajectory.add_auxiliary({"temp": "Temperature"}, reader) + ref_universe.trajectory.add_auxiliary(reader, {"temp": "Temperature"}) ref_dict = get_auxstep_data(0) assert ref_universe.trajectory.ts.aux.temp == ref_dict["Temperature"] -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_terms_update_on_iter(self, ref_universe, reader): - ref_universe.trajectory.add_auxiliary({"bond": "Bond", + ref_universe.trajectory.add_auxiliary(reader, + {"bond": "Bond", "temp": "Temperature"}, - reader) + ) ref_dict = get_auxstep_data(0) assert ref_universe.trajectory.ts.aux.bond == ref_dict["Bond"] assert ref_universe.trajectory.ts.aux.temp == ref_dict["Temperature"] @@ -376,11 +371,11 @@ def test_terms_update_on_iter(self, ref_universe, reader): assert ref_universe.trajectory.ts.aux.bond == ref_dict["Bond"] assert ref_universe.trajectory.ts.aux.temp == ref_dict["Temperature"] -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_invalid_data_selector(self, ref, ref_universe): with pytest.raises(KeyError, match="'Nonsense' is not a key"): - ref_universe.trajectory.add_auxiliary({"something": "Nonsense"}, - AUX_EDR) + ref_universe.trajectory.add_auxiliary(AUX_EDR, + {"something": "Nonsense"}, + ) def test_read_all_times(self, reader): all_times_expected = np.array([0., 0.02, 0.04, 0.06]) @@ -414,20 +409,20 @@ def test_get_data_invalid_selections(self, reader, get_data_input): with pytest.raises(KeyError, match="data selector"): reader.get_data(get_data_input) -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_warning_when_space_in_aux_spec(self, ref_universe, reader): with pytest.warns(UserWarning, match="Auxiliary name"): - ref_universe.trajectory.add_auxiliary({"Pres. DC": "Pres. DC"}, - reader) + ref_universe.trajectory.add_auxiliary(reader, + {"Pres. DC": "Pres. DC"}, + ) -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_warn_too_much_memory_usage(self, ref_universe, reader): with pytest.warns(UserWarning, match="AuxReader: memory usage " "warning! Auxiliary data takes up 3[0-9.]*e-06 GB of" r" memory \(Warning limit: 1e-08 GB\)"): - ref_universe.trajectory.add_auxiliary({"temp": "Temperature"}, - reader, - memory_limit=10) + ref_universe.trajectory.add_auxiliary(reader, + {"temp": "Temperature"}, + memory_limit=10, + ) def test_auxreader_picklable(self, reader): new_reader = pickle.loads(pickle.dumps(reader)) @@ -441,11 +436,11 @@ def test_units_are_converted_by_EDRReader(self, reader): for term in ["Box-X", "Box-Vel-XX"]: assert original_units[term] != reader_units[term] -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 def test_warning_when_unknown_unit(self, ref_universe, reader): with pytest.warns(UserWarning, match="Could not find"): - ref_universe.trajectory.add_auxiliary({"temp": "Temperature"}, - reader) + ref_universe.trajectory.add_auxiliary(reader, + {"temp": "Temperature"}, + ) def test_unit_conversion_is_optional(self, ref): reader = ref.reader( diff --git a/testsuite/MDAnalysisTests/auxiliary/test_xvg.py b/testsuite/MDAnalysisTests/auxiliary/test_xvg.py index 1e9972629e..a10ea5db6d 100644 --- a/testsuite/MDAnalysisTests/auxiliary/test_xvg.py +++ b/testsuite/MDAnalysisTests/auxiliary/test_xvg.py @@ -84,8 +84,7 @@ def ref(): @pytest.fixture def ref_universe(ref): u = mda.Universe(COORDINATES_TOPOLOGY, COORDINATES_XTC) -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 - u.trajectory.add_auxiliary('test', ref.testdata) + u.trajectory.add_auxiliary(ref.testdata, 'test') return u @staticmethod @@ -137,8 +136,7 @@ def ref(): @pytest.fixture def ref_universe(ref): u = mda.Universe(COORDINATES_TOPOLOGY, COORDINATES_XTC) -# TODO: Change order of aux_spec and auxdata for 3.0 release, cf. Issue #3811 - u.trajectory.add_auxiliary('test', ref.testdata) + u.trajectory.add_auxiliary(ref.testdata, 'test') return u @staticmethod diff --git a/testsuite/MDAnalysisTests/coordinates/base.py b/testsuite/MDAnalysisTests/coordinates/base.py index a2a131bbbd..8124279588 100644 --- a/testsuite/MDAnalysisTests/coordinates/base.py +++ b/testsuite/MDAnalysisTests/coordinates/base.py @@ -213,8 +213,8 @@ class BaseReaderTest(object): @pytest.fixture() def reader(ref): reader = ref.reader(ref.trajectory) - reader.add_auxiliary('lowf', ref.aux_lowf, dt=ref.aux_lowf_dt, initial_time=0, time_selector=None) - reader.add_auxiliary('highf', ref.aux_highf, dt=ref.aux_highf_dt, initial_time=0, time_selector=None) + reader.add_auxiliary(ref.aux_lowf, 'lowf', dt=ref.aux_lowf_dt, initial_time=0, time_selector=None) + reader.add_auxiliary(ref.aux_highf, 'highf', dt=ref.aux_highf_dt, initial_time=0, time_selector=None) return reader @staticmethod @@ -303,7 +303,7 @@ def test_iter(self, ref, reader): def test_add_same_auxname_raises_ValueError(self, ref, reader): with pytest.raises(ValueError): - reader.add_auxiliary('lowf', ref.aux_lowf) + reader.add_auxiliary(ref.aux_lowf, 'lowf') def test_remove_auxiliary(self, reader): reader.remove_auxiliary('lowf') @@ -330,7 +330,7 @@ def test_get_aux_attribute(self, ref, reader): def test_iter_as_aux_cutoff(self, ref, reader): # load an auxiliary with the same dt but offset from trajectory, and a # cutoff of 0 - reader.add_auxiliary('offset', ref.aux_lowf, + reader.add_auxiliary(ref.aux_lowf, 'offset', dt=ref.dt, time_selector=None, initial_time=ref.aux_offset_by, cutoff=0) diff --git a/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py b/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py index 5587965a1f..5b05884b54 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py +++ b/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py @@ -69,15 +69,15 @@ def ref(): def reader(self, ref): reader = ChemfilesReader(ref.trajectory) reader.add_auxiliary( - "lowf", ref.aux_lowf, + "lowf", dt=ref.aux_lowf_dt, initial_time=0, time_selector=None, ) reader.add_auxiliary( - "highf", ref.aux_highf, + "highf", dt=ref.aux_highf_dt, initial_time=0, time_selector=None, diff --git a/testsuite/MDAnalysisTests/coordinates/test_copying.py b/testsuite/MDAnalysisTests/coordinates/test_copying.py index b6f42e7aff..020e273b2b 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_copying.py +++ b/testsuite/MDAnalysisTests/coordinates/test_copying.py @@ -294,7 +294,7 @@ def test_positions_share_memory(original_and_copy): def test_copy_with_auxiliary_no_pyedr(): # Check that AuxReaders are copied when reader is copied u = mda.Universe(XYZ_mini) - u.trajectory.add_auxiliary("myaux", AUX_XVG) + u.trajectory.add_auxiliary(AUX_XVG, "myaux") reader = u.trajectory copy = reader.copy() for auxname in reader._auxs: @@ -306,8 +306,8 @@ def test_copy_with_auxiliary_no_pyedr(): def test_copy_with_auxiliary_pyedr(): # Check that AuxReaders are copied when reader is copied u = mda.Universe(XYZ_mini) - u.trajectory.add_auxiliary("myaux", AUX_XVG) - u.trajectory.add_auxiliary({"1": "Bond", "2": "Angle"}, AUX_EDR) + u.trajectory.add_auxiliary(AUX_XVG, "myaux") + u.trajectory.add_auxiliary(AUX_EDR, {"1": "Bond", "2": "Angle"}) reader = u.trajectory copy = reader.copy() diff --git a/testsuite/MDAnalysisTests/coordinates/test_gro.py b/testsuite/MDAnalysisTests/coordinates/test_gro.py index e25bf969fc..86390e3cad 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_gro.py +++ b/testsuite/MDAnalysisTests/coordinates/test_gro.py @@ -285,10 +285,10 @@ def ref(): @pytest.fixture(scope='class') def reader(ref): reader = ref.reader(ref.trajectory, convert_units=False) - reader.add_auxiliary('lowf', ref.aux_lowf, + reader.add_auxiliary(ref.aux_lowf, 'lowf', dt=ref.aux_lowf_dt, initial_time=0, time_selector=None) - reader.add_auxiliary('highf', ref.aux_highf, + reader.add_auxiliary(ref.aux_highf, 'highf', dt=ref.aux_highf_dt, initial_time=0, time_selector=None) return reader