Repository: nismod/smif

Issue:
results - Results.read_scenario_data reads in incorrect data
results.read_scenario_data reads in the wrong data, and the data changes as the list of timesteps passed changes.  Somewhere, there is a misalignment between the Spec coordinates and the data.

For example, actual data:

timestep | lad_uk_2016 | residential_or_non | floor_area
-- | -- | -- | --
2030 | E06000056 | residential | 10841750.0
2030 | E06000056 | non_residential | 6525.1228990099

Using the api
```python
>>> df = results.read_scenario_data(
        'socio-economic', 'arc_baseline', 'floor_area', [2030])
>>> df[df['lad_uk_2016'] == 'E06000056']
```
returns the incorrected data:

timestep | lad_uk_2016 | residential_or_non | floor_area
-- | -- | -- | --
2030 | E06000056 | residential | 3424711.285
2030 | E06000056 | non_residential | 4647666.550

Direct read from the store gives expected data:

```python
>>> store = results._store
>>> df = store.read_scenario_variant_data('socio-economic', 'arc_baseline', 'floor_area', 2030).as_df().reset_index()
>>> df[df['lad_uk_2016'] == 'E06000056']
```

lad_uk_2016 | residential_or_non | floor_area
-- | -- | --
E06000056 | residential | 1.084175e+07
E06000056 | non_residential | 6.525123e+03



Discussion:

Patch:
diff --git a/src/smif/data_layer/store.py b/src/smif/data_layer/store.py
index 81dfd898..5ae27a54 100644
--- a/src/smif/data_layer/store.py
+++ b/src/smif/data_layer/store.py
@@ -23,6 +23,7 @@ from operator import itemgetter
 from typing import Dict, List, Optional
 
 import numpy as np  # type: ignore
+
 from smif.data_layer import DataArray
 from smif.data_layer.abstract_data_store import DataStore
 from smif.data_layer.abstract_metadata_store import MetadataStore
@@ -536,7 +537,6 @@ class Store():
         variant_name : str
         variable : str
         timestep : int
-            If None, read data for all timesteps
 
         Returns
         -------
@@ -907,13 +907,13 @@ class Store():
         # see store.read_scenario_variant_data
         scenario = self.read_scenario(scenario_name)
         spec_dict = _pick_from_list(scenario['provides'], variable)
-        
+
         # Now append timestep dimension, see store.get_result_darray_internal()
-        spec_dict['dims'].append('timestep')
+        spec_dict['dims'] = ['timestep'] + spec_dict['dims']
         spec_dict['coords']['timestep'] = timesteps
-        
+
         spec = Spec.from_dict(spec_dict)
-                              
+
         # Read the results for each timestep tuple and stack them
         list_of_numpy_arrays = []
         for t in timesteps:
@@ -921,10 +921,10 @@ class Store():
             list_of_numpy_arrays.append(d_array.data)
 
         stacked_data = np.vstack(list_of_numpy_arrays)
-        data = np.transpose(stacked_data)
-        
+        data = stacked_data
+
         return DataArray(spec, np.reshape(data, spec.shape))
-        
+
     def available_results(self, model_run_name):
         """List available results from a model run
 


Test patch:
diff --git a/tests/data_layer/test_data_store.py b/tests/data_layer/test_data_store.py
index 55c25cbb..f03df7e4 100644
--- a/tests/data_layer/test_data_store.py
+++ b/tests/data_layer/test_data_store.py
@@ -4,6 +4,7 @@ from copy import deepcopy
 
 import numpy as np
 from pytest import fixture, mark, param, raises
+
 from smif.data_layer.data_array import DataArray
 from smif.data_layer.database_interface import DbDataStore
 from smif.data_layer.file.file_data_store import CSVDataStore, ParquetDataStore
diff --git a/tests/data_layer/test_store.py b/tests/data_layer/test_store.py
index 36250135..5f0a640e 100644
--- a/tests/data_layer/test_store.py
+++ b/tests/data_layer/test_store.py
@@ -6,6 +6,7 @@ cross-coordination and there are some convenience methods implemented at this la
 import numpy as np
 import numpy.testing
 from pytest import fixture, raises
+
 from smif.data_layer import Store
 from smif.data_layer.data_array import DataArray
 from smif.data_layer.memory_interface import (MemoryConfigStore,
@@ -197,15 +198,10 @@ class TestStoreMetadata():
 
 
 class TestStoreData():
-    def test_scenario_variant_data(self, store, sample_dimensions, scenario,
-                                   sample_scenario_data):
-        # The sample_scenario_data fixture provides data with a spec including timestep
-        # dimension containing a single coordinate of 2015. Note the asymmetry in the write
-        # and read methods here: writing requires the full DataArray object with the full
-        # spec including timestep, but the reading requires a specific timestep to be supplied.
-        # The data read back in, therefore, has lower dimensionality.
 
-        # setup
+    @fixture(scope='function')
+    def setup(self, store, sample_dimensions, scenario,
+              sample_scenario_data):
         for dim in sample_dimensions:
             store.write_dimension(dim)
         store.write_scenario(scenario)
@@ -217,6 +213,18 @@ class TestStoreData():
         store.write_scenario_variant_data(
             scenario_name, variant_name, scenario_variant_data
         )
+        return key, scenario_variant_data
+
+    def test_scenario_variant_data(self, store,
+                                   setup):
+        # The sample_scenario_data fixture provides data with a spec including timestep
+        # dimension containing a single coordinate of 2015. Note the asymmetry in the write
+        # and read methods here: writing requires the full DataArray object with the full
+        # spec including timestep, but the reading requires a specific timestep to be supplied.
+        # The data read back in, therefore, has lower dimensionality.
+
+        key, scenario_variant_data = setup
+        scenario_name, variant_name, variable = key
 
         # Read 2015
         actual = store.read_scenario_variant_data(
@@ -230,6 +238,31 @@ class TestStoreData():
         )
         assert (actual.data == scenario_variant_data.data[1]).all()
 
+    def test_scenario_variant_data_mult_one_year(self, store, setup):
+        key, scenario_variant_data = setup
+        scenario_name, variant_name, variable = key
+
+        actual = store.read_scenario_variant_data_multiple_timesteps(
+            scenario_name, variant_name, variable, [2016]
+        )
+
+        print(actual.as_df())
+        print(scenario_variant_data.as_df())
+
+        assert (actual.data == [scenario_variant_data.data[1]]).all()
+
+    def test_scenario_variant_data_mult_mult_years(self, store, setup):
+
+        key, scenario_variant_data = setup
+        scenario_name, variant_name, variable = key
+
+        actual = store.read_scenario_variant_data_multiple_timesteps(
+            scenario_name, variant_name, variable, [2015, 2016]
+        )
+        print(actual.as_df())
+        print(scenario_variant_data.as_df())
+        assert (actual.data == scenario_variant_data.data).all()
+
     def test_narrative_variant_data(self, store, sample_dimensions, get_sos_model,
                                     get_sector_model, energy_supply_sector_model,
                                     sample_narrative_data):