Repository: spinnakermanchester/spynnaker
Issue: Population Recorder split will need a second pass

https://github.com/SpiNNakerManchester/sPyNNaker/pull/958 did some good work splitting it so A population has a recorder 

After the merge a second pass would be good to make sure all the cleanup was done.

For example
population.__record_spike_file ect wehre and still are never used.

Changed files:
spynnaker/pyNN/models/populations/population.py, spynnaker/pyNN/models/populations/population_view.py, unittests/test_pop_views_assembly/test_record.py, spynnaker/pyNN/models/recorder.py, unittests/test_using_virtual_board/test_set_record.py

diff --git a/spynnaker/pyNN/models/populations/population.py b/spynnaker/pyNN/models/populations/population.py
index 1cade523fe..831644e3ea 100644
--- a/spynnaker/pyNN/models/populations/population.py
+++ b/spynnaker/pyNN/models/populations/population.py
@@ -71,9 +71,6 @@ class Population(PopulationBase):
         "__last_id",
         "_positions",
         "_recorder",
-        "__record_gsyn_file",
-        "__record_spike_file",
-        "__record_v_file",
         "_size",
         "__structure",
         "__vertex",
@@ -131,9 +128,6 @@ class Population(PopulationBase):
             size = self.__vertex.n_atoms
         self._size = size
         self._annotations = dict()
-        self.__record_spike_file = None
-        self.__record_v_file = None
-        self.__record_gsyn_file = None
 
         # parameter
         self.__change_requires_mapping = True
@@ -200,8 +194,7 @@ class Population(PopulationBase):
         return variable in \\
             self._recorder.get_all_possible_recordable_variables()
 
-    def record(self, variables, to_file=None, sampling_interval=None,
-               indexes=None):
+    def record(self, variables, to_file=None, sampling_interval=None):
         """ Record the specified variable or variables for all cells in the\\
             Population or view.
 
@@ -215,26 +208,15 @@ class Population(PopulationBase):
         :type to_file: ~neo.io or ~neo.rawio or str
         :param int sampling_interval: a value in milliseconds, and an integer
             multiple of the simulation timestep.
-        :param indexes: The indexes of neurons to record from.
-            This is non-standard PyNN and equivalent to creating a view with
-            these indexes and asking the View to record.
-        :type indexes: None or list(int)
         """
         # pylint: disable=arguments-differ
-        if indexes is not None:
-            warn_once(
-                logger, "record indexes parameter is non-standard PyNN, "
-                "so may not be portable to other simulators. "
-                "It is now deprecated and replaced with views")
-        self._record_with_indexes(
-            variables, to_file, sampling_interval, indexes)
+        self._recorder.record(
+            variables, to_file, sampling_interval, indexes=None)
 
-    def _record_with_indexes(
+    def _record(
             self, variables, to_file, sampling_interval, indexes):
-        """ Same as record but without non-standard PyNN warning
-
-        This method is non-standard PyNN and is intended only to be called by
-        record in a Population, View or Assembly
+        """ Record the specified variable or variables for all cells in the\\
+            Population or view.
 
         :param variables: either a single variable name or a list of variable
             names. For a given celltype class, ``celltype.recordable`` contains
@@ -252,44 +234,8 @@ class Population(PopulationBase):
             these indexes and asking the View to record.
         :type indexes: None or list(int)
         """
-        if variables is None:  # reset the list of things to record
-            if sampling_interval is not None:
-                raise ConfigurationException(
-                    "Clash between parameters in record."
-                    "variables=None turns off recording,"
-                    "while sampling_interval!=None implies turn on recording")
-            if indexes is not None:
-                warn_once(
-                    logger,
-                    "View.record with variable None is non-standard PyNN. "
-                    "Only the neurons in the view have their record turned "
-                    "off. Other neurons already set to record will remain "
-                    "set to record")
-
-            # note that if record(None) is called, its a reset
-            self._recorder.turn_off_all_recording(indexes)
-            # handle one element vs many elements
-        elif isinstance(variables, string_types):
-            # handle special case of 'all'
-            if variables == "all":
-                warn_once(
-                    logger, 'record("all") is non-standard PyNN, and '
-                    'therefore may not be portable to other simulators.')
-
-                # iterate though all possible recordings for this vertex
-                for variable in self._recorder.\\
-                        get_all_possible_recordable_variables():
-                    self._recorder.record(
-                        variable, sampling_interval, to_file, indexes)
-            else:
-                # record variable
-                self._recorder.record(
-                    variables, sampling_interval, to_file, indexes)
-
-        else:  # list of variables, so just iterate though them
-            for variable in variables:
-                self._recorder.record(
-                    variable, sampling_interval, to_file, indexes)
+        self._recorder.record(
+            variables, to_file, sampling_interval, indexes=indexes)
 
     def sample(self, n, rng=None):
         """ Randomly sample `n` cells from the Population, and return a\\
diff --git a/spynnaker/pyNN/models/populations/population_view.py b/spynnaker/pyNN/models/populations/population_view.py
index 5b5eeb302a..4717857bab 100644
--- a/spynnaker/pyNN/models/populations/population_view.py
+++ b/spynnaker/pyNN/models/populations/population_view.py
@@ -413,7 +413,7 @@ class PopulationView(PopulationBase):
             should be a value in milliseconds, and an integer multiple of the
             simulation timestep.
         """
-        self.__population._record_with_indexes(
+        self.__population._record(
             variables, to_file, sampling_interval, self.__indexes)
 
     def sample(self, n, rng=None):
diff --git a/spynnaker/pyNN/models/recorder.py b/spynnaker/pyNN/models/recorder.py
index 145c875fc7..099b236263 100644
--- a/spynnaker/pyNN/models/recorder.py
+++ b/spynnaker/pyNN/models/recorder.py
@@ -78,8 +78,69 @@ class Recorder(object):
         """
         return self.__write_to_files_indicators
 
-    def record(self, variable, sampling_interval=None, to_file=None,
-               indexes=None):
+    def record(
+            self, variables, to_file, sampling_interval, indexes):
+        """ Same as record but without non-standard PyNN warning
+
+        This method is non-standard PyNN and is intended only to be called by
+        record in a Population, View or Assembly
+
+        :param variables: either a single variable name or a list of variable
+            names. For a given celltype class, ``celltype.recordable`` contains
+            a list of variables that can be recorded for that celltype.
+            Can also be ``None`` to reset the list of variables.
+        :type variables: str or list(str) or None
+        :param to_file: a file to automatically record to (optional).
+            :py:meth:`write_data` will be automatically called when
+            `sim.end()` is called.
+        :type to_file: ~neo.io or ~neo.rawio or str
+        :param int sampling_interval: a value in milliseconds, and an integer
+            multiple of the simulation timestep.
+        :param indexes: The indexes of neurons to record from.
+            This is non-standard PyNN and equivalent to creating a view with
+            these indexes and asking the View to record.
+        :type indexes: None or list(int)
+        """
+        if variables is None:  # reset the list of things to record
+            if sampling_interval is not None:
+                raise ConfigurationException(
+                    "Clash between parameters in record."
+                    "variables=None turns off recording,"
+                    "while sampling_interval!=None implies turn on recording")
+            if indexes is not None:
+                warn_once(
+                    logger,
+                    "View.record with variable None is non-standard PyNN. "
+                    "Only the neurons in the view have their record turned "
+                    "off. Other neurons already set to record will remain "
+                    "set to record")
+
+            # note that if record(None) is called, its a reset
+            self.turn_off_all_recording(indexes)
+            # handle one element vs many elements
+        elif isinstance(variables, string_types):
+            # handle special case of 'all'
+            if variables == "all":
+                warn_once(
+                    logger, 'record("all") is non-standard PyNN, and '
+                    'therefore may not be portable to other simulators.')
+
+                # iterate though all possible recordings for this vertex
+                for variable in self.get_all_possible_recordable_variables():
+                    self.turn_on_record(
+                        variable, sampling_interval, to_file, indexes)
+            else:
+                # record variable
+                self.turn_on_record(
+                    variables, sampling_interval, to_file, indexes)
+
+        else:  # list of variables, so just iterate though them
+            for variable in variables:
+                self.turn_on_record(
+                    variable, sampling_interval, to_file, indexes)
+
+    def turn_on_record(self, variable, sampling_interval=None, to_file=None,
+                       indexes=None):
         """ Tell the vertex to record data.
 
         :param str variable: The variable to record, supported variables to
diff --git a/unittests/test_pop_views_assembly/test_record.py b/unittests/test_pop_views_assembly/test_record.py
index 57afcb7a11..c669cb445a 100644
--- a/unittests/test_pop_views_assembly/test_record.py
+++ b/unittests/test_pop_views_assembly/test_record.py
@@ -86,7 +86,8 @@ class TestPopulation(BaseTestCase):
     def test_record_with_indexes(self):
         sim.setup(timestep=1.0)
         pop = sim.Population(10, sim.IF_curr_exp())
-        pop.record("v", indexes=[2, 3, 4])
+        pop._record(
+            "v", to_file=None, sampling_interval=None, indexes=[2, 3, 4])
         target = {"v"}
         assert target == set(pop._recorder.get_all_recording_variables())
         target1 = [2, 3, 4]
diff --git a/unittests/test_using_virtual_board/test_set_record.py b/unittests/test_using_virtual_board/test_set_record.py
index e5f5d7e8fe..8005962ce6 100644
--- a/unittests/test_using_virtual_board/test_set_record.py
+++ b/unittests/test_using_virtual_board/test_set_record.py
@@ -116,9 +116,9 @@ class TestSetRecord(BaseTestCase):
             5, sim.SpikeSourceArray(spike_times=[0]))
         ssp = sim.Population(5, sim.SpikeSourcePoisson(rate=100.0),
                              additional_parameters={"seed": 1})
-        if_curr.record("spikes", indexes=[1, 2, 4])
-        ssa.record("spikes", indexes=[1, 2, 4])
-        ssp.record("spikes", indexes=[1, 2, 4])
+        if_curr[1, 2, 4].record("spikes")
+        ssa[1, 2, 4].record("spikes")
+        ssp[1, 2, 4].record("spikes")
         self.assertListEq(["spikes"],
                           if_curr._recorder.get_all_recording_variables())
         assert recorder._indexes["spikes"] == [1, 2, 4]
@@ -127,8 +127,8 @@ class TestSetRecord(BaseTestCase):
         sim.setup(timestep=1)
         if_curr = sim.Population(5, sim.IF_curr_exp())
         recorder = if_curr._vertex.neuron_recorder
-        if_curr.record("spikes", indexes=[1, 2, 4])
-        if_curr.record("spikes", indexes=[1, 3])
+        if_curr[1, 2, 4].record("spikes")
+        if_curr[1, 3].record("spikes")
         self.assertListEq(["spikes"],
                           if_curr._recorder.get_all_recording_variables())
         assert recorder._indexes["spikes"] == [1, 2, 3, 4]