Bug fixes and improve tests

Author: enekomartinmartinez

docs/whats_new.rst

@@ -19,6 +19,7 @@ Bug fixes
 - Fix bug when a WITH LOOKUPS argument has subscripts. (`@enekomartinmartinez <https://github.com/enekomartinmartinez>`_)
 - Fix bug of exportig csv files with multiple subscripts variables. (`@rogersamso <https://github.com/rogersamso>`_)
 - Fix bug of missing dimensions in variables defined with not all the subscripts of a range (:issue:`364`). (`@enekomartinmartinez <https://github.com/enekomartinmartinez>`_)
+- Fix bug when running a model with variable final time or time step and progressbar (:issue:`361`). (`@enekomartinmartinez <https://github.com/enekomartinmartinez>`_)
 
 Documentation
 ~~~~~~~~~~~~~
@@ -31,7 +32,9 @@ Performance
 Internal Changes
 ~~~~~~~~~~~~~~~~
 - Make PySD work with :py:mod:`parsimonius` 0.10.0. (`@enekomartinmartinez <https://github.com/enekomartinmartinez>`_)
-- Add netCDF4 and hdf5 dependencies. (`@rogersamso <https://github.com/rogersamso>`_)
+- Add netCDF4 dependency for tests. (`@rogersamso <https://github.com/rogersamso>`_)
+- Improve warning message when replacing a stock with a parameter.  (`@enekomartinmartinez <https://github.com/enekomartinmartinez>`_)
+- Include more pytest parametrizations in some test and make them translate the models in temporary directories.  (`@enekomartinmartinez <https://github.com/enekomartinmartinez>`_)
 
 
 v3.6.1 (2022/09/05)

pysd/py_backend/model.py

@@ -485,7 +485,7 @@ def export(self, file_name):
 
         Parameters
         ----------
-        file_name: str
+        file_name: str or pathlib.Path
           Name of the file to export the values.
 
         """
@@ -512,7 +512,7 @@ def import_pickle(self, file_name):
 
         Parameters
         ----------
-        file_name: str
+        file_name: str or pathlib.Path
           Name of the file to import the values from.
 
         """
@@ -781,8 +781,8 @@ def set_components(self, params, new=False):
 
             # this won't handle other statefuls...
             if '_integ_' + func_name in dir(self.components):
-                warnings.warn("Replacing the equation of stock"
-                              + "{} with params".format(key),
+                warnings.warn("Replacing the equation of stock "
+                              "'{}' with params...".format(key),
                               stacklevel=2)
 
             new_function.__name__ = func_name
@@ -1134,8 +1134,6 @@ def run(self, params=None, return_columns=None, return_timestamps=None,
         if reload:
             self.reload()
 
-        self.progress = progress
-
         self.time.add_return_timestamps(return_timestamps)
         if self.time.return_timestamps is not None and not final_time:
             # if not final time given the model will end in the list
@@ -1156,6 +1154,18 @@ def run(self, params=None, return_columns=None, return_timestamps=None,
 
         self.set_initial_condition(initial_condition)
 
+        # set progressbar
+        if progress and (self.cache_type["final_time"] == "step" or
+                         self.cache_type["time_step"] == "step"):
+            warnings.warn(
+                "The progressbar is not compatible with dynamic "
+                "final time or time step. Both variables must be "
+                "constants to prompt progress."
+            )
+            progress = False
+
+        self.progress = progress
+
         if return_columns is None or isinstance(return_columns, str):
             return_columns = self._default_return_columns(return_columns)
 
@@ -1625,7 +1635,7 @@ def set_initial_condition(self, initial_condition):
 
         Parameters
         ----------
-        initial_condition : str or (float, dict)
+        initial_condition : str or (float, dict) or pathlib.Path
             The starting time, and the state of the system (the values of
             all the stocks) at that starting time. 'original' or 'o'uses
             model-file specified initial condition. 'current' or 'c' uses
@@ -1647,19 +1657,21 @@ def set_initial_condition(self, initial_condition):
         model.set_initial_value()
 
         """
+        if isinstance(initial_condition, str)\
+           and initial_condition.lower() not in ["original", "o",
+                                                 "current", "c"]:
+            initial_condition = Path(initial_condition)
 
         if isinstance(initial_condition, tuple):
             self.initialize()
             self.set_initial_value(*initial_condition)
+        elif isinstance(initial_condition, Path):
+            self.import_pickle(initial_condition)
         elif isinstance(initial_condition, str):
             if initial_condition.lower() in ["original", "o"]:
                 self.time.set_control_vars(
                     initial_time=self.components._control_vars["initial_time"])
                 self.initialize()
-            elif initial_condition.lower() in ["current", "c"]:
-                pass
-            else:
-                self.import_pickle(initial_condition)
         else:
             raise TypeError(
                 "Invalid initial conditions. "

tests/conftest.py

@@ -1,6 +1,13 @@
-import pytest
+import shutil
 from pathlib import Path
 
+import pytest
+from pysd import read_vensim, read_xmile, load
+from pysd.translators.vensim.vensim_utils import supported_extensions as\
+    vensim_extensions
+from pysd.translators.xmile.xmile_utils import supported_extensions as\
+    xmile_extensions
+
 
 @pytest.fixture(scope="session")
 def _root():
@@ -18,3 +25,24 @@ def _test_models(_root):
 def shared_tmpdir(tmpdir_factory):
     # shared temporary directory for each class
     return Path(tmpdir_factory.mktemp("shared"))
+
+
+@pytest.fixture
+def model(_root, tmp_path, model_path):
+    """
+    Copy model to the tmp_path and translate it
+    """
+    assert (_root / model_path).exists(), "The model doesn't exist"
+
+    target = tmp_path / model_path.parent.name
+    new_path = target / model_path.name
+    shutil.copytree(_root / model_path.parent, target)
+
+    if model_path.suffix.lower() in vensim_extensions:
+        return read_vensim(new_path)
+    elif model_path.suffix.lower() in xmile_extensions:
+        return read_xmile(new_path)
+    elif model_path.suffix.lower() == ".py":
+        return load(new_path)
+    else:
+        return ValueError("Invalid model")

tests/pytest_pysd/pytest_output.py

@@ -1,5 +1,4 @@
 from pathlib import Path
-import shutil
 
 import pytest
 import numpy as np
@@ -11,8 +10,6 @@
 from pysd.py_backend.output import OutputHandlerInterface, DatasetHandler, \
     DataFrameHandler, ModelOutput
 
-import pysd
-
 
 test_model_look = Path(
     "test-models/tests/get_lookups_subscripted_args/"
@@ -36,18 +33,6 @@
 )
 
 
-@pytest.fixture
-@pytest.mark.filterwarnings("ignore")
-def model(_root, tmp_path, model_path):
-    """
-    Copy model to the tmp_path and translate it
-    """
-
-    target = tmp_path / model_path.parent
-    shutil.copytree(_root / model_path.parent, target)
-    return pysd.read_vensim(target / model_path.name)
-
-
 class TestOutput():
 
     def test_output_handler_interface(self):
@@ -135,6 +120,16 @@ class EmptyHandler(OutputHandlerInterface):
             EmptyHandler.add_run_elements(
                 EmptyHandler, "model", "capture")
 
+    @pytest.mark.parametrize("model_path", [test_model_look])
+    def test_invalid_output_file(self, model):
+        error_message = "Paths must be strings or pathlib Path objects."
+        with pytest.raises(TypeError, match=error_message):
+            model.run(output_file=1234)
+
+        error_message = "Unsupported output file format .txt"
+        with pytest.raises(ValueError, match=error_message):
+            model.run(output_file="file.txt")
+
     @pytest.mark.parametrize(
         "model_path,dims,values",
         [