Partial Fix: Change order of arguments for adding auxiliary data for 3.0 release (Issue #3811)

package/AUTHORS

@@ -236,6 +236,7 @@ Chronological list of authors
   2024
   - Aditya Keshari
   - Philipp Stärk
+  - Valerij Talagayev
 
 External code
 -------------

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 

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

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

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

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

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):

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,65 +308,61 @@ 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
         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_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(

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

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)

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,

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()

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