Merge pull request #282 from enekomartinmartinez/close_excels

Close files opened with openpyxl
Correct bug when subseting ranges
Correct bug of dynamic final time

Author: enekomartinmartinez

.travis.yml

@@ -8,6 +8,7 @@ install:
   - pip install cython
   - pip install --upgrade pip setuptools wheel
   - pip install -e .
+  - pip install psutil
   - pip install pytest pytest-cov
   - pip install coveralls
 # command to run tests

pysd/_version.py

@@ -1 +1 @@
-__version__ = "1.8.0"
+__version__ = "1.8.1"

pysd/py_backend/external.py

@@ -53,6 +53,10 @@ def clean(cls):
         """
         Clean the dictionary of read files
         """
+        for file in cls._Excels_opyxl.values():
+            # close files open directly with openpyxls
+            file.close()
+            # files open with pandas are automatically closed
         cls._Excels, cls._Excels_opyxl = {}, {}
 
 

pysd/py_backend/functions.py

@@ -1243,13 +1243,17 @@ def _build_euler_timeseries(self, return_timestamps=None, final_time=None):
         except IndexError:
             # return_timestamps is an empty list
             # model default final time or passed argument value
-            t_f = self.final_time
+            t_f = self.components.final_time()
 
         if final_time is not None:
             t_f = max(final_time, t_f)
 
         ts = np.arange(
-            t_0, t_f+self.time_step/2, self.time_step, dtype=np.float64)
+            t_0,
+            t_f+self.components.time_step()/2,
+            self.components.time_step(),
+            dtype=np.float64
+        )
 
         # Add the returned time series into the integration array.
         # Best we can do for now. This does change the integration ever
@@ -1278,11 +1282,10 @@ def _format_return_timestamps(self, return_timestamps=None):
             # Vensim's standard is to expect that the data set includes
             # the `final time`, so we have to add an extra period to
             # make sure we get that value in what numpy's `arange` gives us.
-
             return np.arange(
                 self.time(),
-                self.final_time + self.saveper/2,
-                self.saveper, dtype=float
+                self.components.final_time() + self.components.saveper()/2,
+                self.components.saveper(), dtype=float
             )
 
         try:
@@ -1377,23 +1380,35 @@ def run(self, params=None, return_columns=None, return_timestamps=None,
 
         self.progress = progress
 
+        # TODO move control variables to a class
+        if params is None:
+            params = {}
+        if final_time:
+            params['final_time'] = final_time
+        elif return_timestamps is not None:
+            params['final_time'] =\
+                self._format_return_timestamps(return_timestamps)[-1]
+        if time_step:
+            params['time_step'] = time_step
+        if saveper:
+            params['saveper'] = saveper
+        # END TODO
+
         if params:
             self.set_components(params)
 
         self.set_initial_condition(initial_condition)
 
+        # TODO move control variables to a class
         # save control variables
-        self.initial_time = self.time()
-        self.final_time = final_time or self.components.final_time()
-        self.time_step = time_step or self.components.time_step()
-        self.saveper = saveper or max(self.time_step,
-                                      self.components.saveper())
-        # need to take bigger saveper if time_step is > saveper
+        replace = {
+            'initial_time': self.time()
+        }
+        # END TODO
 
         return_timestamps = self._format_return_timestamps(return_timestamps)
 
         t_series = self._build_euler_timeseries(return_timestamps, final_time)
-        self.final_time = t_series[-1]
 
         if return_columns is None or isinstance(return_columns, str):
             return_columns = self._default_return_columns(return_columns)
@@ -1410,10 +1425,9 @@ def run(self, params=None, return_columns=None, return_timestamps=None,
         res = self._integrate(t_series, capture_elements['step'],
                               return_timestamps)
 
-        self._add_run_elements(res, capture_elements['run'])
+        self._add_run_elements(res, capture_elements['run'], replace=replace)
 
         return_df = utils.make_flat_df(res, return_addresses, flatten_output)
-        return_df.index = return_timestamps
 
         return return_df
 
@@ -1562,8 +1576,7 @@ def _integrate(self, time_steps, capture_elements, return_timestamps):
         outputs: list of dictionaries
 
         """
-        outputs = pd.DataFrame(index=return_timestamps,
-                               columns=capture_elements)
+        outputs = pd.DataFrame(columns=capture_elements)
 
         if self.progress:
             # initialize progress bar
@@ -1580,6 +1593,10 @@ def _integrate(self, time_steps, capture_elements, return_timestamps):
             self.time.update(t2)  # this will clear the stepwise caches
             self.components.cache.reset(t2)
             progressbar.update()
+            # TODO move control variables to a class and automatically stop
+            # when updating time
+            if self.time() >= self.components.final_time():
+                break
 
         # need to add one more time step, because we run only the state
         # updates in the previous loop and thus may be one short.
@@ -1591,7 +1608,7 @@ def _integrate(self, time_steps, capture_elements, return_timestamps):
 
         return outputs
 
-    def _add_run_elements(self, df, capture_elements):
+    def _add_run_elements(self, df, capture_elements, replace={}):
         """
         Adds constant elements to a dataframe.
 
@@ -1603,6 +1620,10 @@ def _add_run_elements(self, df, capture_elements):
         capture_elements: list
             List of constant elements
 
+        replace: dict
+            Ouputs values to replace.
+            TODO: move control variables to a class and avoid this.
+
         Returns
         -------
         None
@@ -1612,16 +1633,17 @@ def _add_run_elements(self, df, capture_elements):
         for element in capture_elements:
             df[element] = [getattr(self.components, element)()] * nt
 
+        # TODO: move control variables to a class and avoid this.
         # update initial time values in df (necessary if initial_conditions)
-        for it in ['initial_time', 'final_time', 'saveper', 'time_step']:
+        for it, value in replace.items():
             if it in df:
-                df[it] = getattr(self, it)
+                df[it] = value
             elif it.upper() in df:
-                df[it.upper()] = getattr(self, it)
+                df[it.upper()] = value
             elif it.replace('_', ' ') in df:
-                df[it.replace('_', ' ')] = getattr(self, it)
+                df[it.replace('_', ' ')] = value
             elif it.replace('_', ' ').upper() in df:
-                df[it.replace('_', ' ').upper()] = getattr(self, it)
+                df[it.replace('_', ' ').upper()] = value
 
 
 def ramp(time, slope, start, finish=0):

pysd/py_backend/utils.py

@@ -11,6 +11,7 @@
 
 import regex as re
 import progressbar
+import numpy as np
 import xarray as xr
 
 # used to create python safe names
@@ -688,9 +689,7 @@ def rearrange(data, dims, coords):
         if data.shape == shape:
             # Allows switching dimensions names and transpositions
             return xr.DataArray(data=data.values, coords=coords, dims=dims)
-        elif len(shape) == len(data.shape) and all(
-            [shape[i] < data.shape[i] for i in range(len(shape))]
-        ):
+        elif np.prod(shape) < np.prod(data.shape):
             # Allows subscripting a subrange
             return data.rename(
                 {dim: new_dim for dim, new_dim in zip(data.dims, dims)}

tests/integration_test_vensim_pathway.py

@@ -50,7 +50,6 @@ def test_delay_fixed(self):
             output, canon = runner('test-models/tests/delay_fixed/test_delay_fixed.mdl')
         assert_frames_close(output, canon, rtol=rtol)
 
-    @unittest.skip('to be fixed #225')
     def test_delay_numeric_error(self):
         # issue https://github.com/JamesPHoughton/pysd/issues/225
         output, canon = runner('test-models/tests/delay_numeric_error/test_delay_numeric_error.mdl')
@@ -72,6 +71,11 @@ def test_delays(self):
         output, canon = runner('test-models/tests/delays/test_delays.mdl')
         assert_frames_close(output, canon, rtol=rtol)
 
+    def test_dynamic_final_time(self):
+        # issue https://github.com/JamesPHoughton/pysd/issues/278
+        output, canon = runner('test-models/tests/dynamic_final_time/test_dynamic_final_time.mdl')
+        assert_frames_close(output, canon, rtol=rtol)
+
     def test_euler_step_vs_saveper(self):
         output, canon = runner('test-models/tests/euler_step_vs_saveper/test_euler_step_vs_saveper.mdl')
         assert_frames_close(output, canon, rtol=rtol)

tests/test-models

@@ -66,6 +66,33 @@ def test_read_clean_opyxl(self):
         self.assertEqual(list(pysd.external.Excels._Excels_opyxl),
                          [])
 
+    def test_close_file(self):
+        """
+        Test for checking if excel files were closed
+        """
+        import pysd
+        import psutil
+
+        p = psutil.Process()
+
+        # number of files already open
+        n_files = len(p.open_files())
+
+        file_name = "data/input.xlsx"
+        sheet_name = "Vertical"
+        sheet_name2 = "Horizontal"
+
+        # reading files
+        pysd.external.Excels.read(file_name, sheet_name)
+        pysd.external.Excels.read(file_name, sheet_name2)
+        pysd.external.Excels.read_opyxl(file_name)
+
+        self.assertGreater(len(p.open_files()), n_files)
+
+        # clean
+        pysd.external.Excels.clean()
+        self.assertEqual(len(p.open_files()), n_files)
+
 
 class TestExternalMethods(unittest.TestCase):
     """
@@ -3083,7 +3110,6 @@ def test_data_interp_hnnm_keep(self):
 
         self.assertTrue(data.data.equals(expected))
 
-
     def test_lookup_data_attr(self):
         """
         Test for keep in series when the series is not
@@ -3119,4 +3145,4 @@ def test_lookup_data_attr(self):
         datL.initialize()
 
         self.assertTrue(hasattr(datD, 'time_row_or_cols'))
-        self.assertTrue(hasattr(datL, 'x_row_or_cols'))
+        self.assertTrue(hasattr(datL, 'x_row_or_cols'))

tests/unit_test_external.py

@@ -1590,8 +1590,8 @@ def test__build_euler_timeseries(self):
 
         model = pysd.read_vensim(test_model)
         model.components.initial_time = lambda: 3
-        model.final_time = 50
-        model.time_step = 1
+        model.components.final_time = lambda: 50
+        model.components.time_step = lambda: 1
         model.initialize()
 
         actual = list(model._build_euler_timeseries(return_timestamps=[10]))

tests/unit_test_pysd.py

@@ -438,11 +438,16 @@ def test_rearrange(self):
         _subscript_dict = {
             'd1': ['a', 'b', 'c'],
             'd2': ['b', 'c'],
-            'd3': ['b', 'c']
+            'd3': ['b', 'c'],
+            'd4': ['e', 'f']
         }
 
         xr_input_subdim = xr.DataArray([1, 4, 2],
                                        {'d1': ['a', 'b', 'c']}, ['d1'])
+        xr_input_subdim2 = xr.DataArray([[1, 4, 2], [3, 4, 5]],
+                                        {'d1': ['a', 'b', 'c'],
+                                         'd4': ['e', 'f']},
+                                        ['d4', 'd1'])
         xr_input_updim = xr.DataArray([1, 4],
                                       {'d2': ['b', 'c']}, ['d2'])
         xr_input_switch = xr.DataArray([[1, 4], [8, 5]],
@@ -452,6 +457,10 @@ def test_rearrange(self):
 
         xr_out_subdim = xr.DataArray([4, 2],
                                      {'d2': ['b', 'c']}, ['d2'])
+        xr_out_subdim2 = xr.DataArray([[4, 2], [4, 5]],
+                                      {'d2': ['b', 'c'],
+                                       'd4': ['e', 'f']},
+                                      ['d4', 'd2'])
         xr_out_updim = xr.DataArray([[1, 1], [4, 4]],
                                     {'d2': ['b', 'c'], 'd3': ['b', 'c']},
                                     ['d2', 'd3'])
@@ -464,6 +473,9 @@ def test_rearrange(self):
         self.assertTrue(xr_out_subdim.equals(
             rearrange(xr_input_subdim, ['d2'], _subscript_dict)))
 
+        self.assertTrue(xr_out_subdim2.equals(
+            rearrange(xr_input_subdim2, ['d4', 'd2'], _subscript_dict)))
+
         self.assertTrue(xr_out_updim.equals(
             rearrange(xr_input_updim, ['d2', 'd3'], _subscript_dict)))