Make getters raise KeyError instead of returning "NotExist" (#299)

* Make getters raise KeyError

... instead of returning weird values like "NotExist"

* Fix incorrect use of IOError

When IOError constructor is called with two arguments, the first
argument is interpreted as the error number:
In [1]: str(IOError("/test1", "does not exist"))
Out[1]: '[Errno /test1] does not exist'

* Improve docstring for ModelicaSystem.getQuantities

* Silence DeprecationWarning in tests

Author: ondras12345

OMPython/ModelicaSystem.py

@@ -472,12 +472,12 @@ def setTempDirectory(self, customBuildDirectory: Optional[str | os.PathLike | pa
         # create a unique temp directory for each session and build the model in that directory
         if customBuildDirectory is not None:
             if not os.path.exists(customBuildDirectory):
-                raise IOError(customBuildDirectory, " does not exist")
+                raise IOError(f"{customBuildDirectory} does not exist")
             tempdir = pathlib.Path(customBuildDirectory)
         else:
             tempdir = pathlib.Path(tempfile.mkdtemp())
             if not tempdir.is_dir():
-                raise IOError(tempdir, " cannot be created")
+                raise IOError(f"{tempdir} could not be created")
 
         logger.info("Define tempdir as %s", tempdir)
         exp = f'cd("{tempdir.absolute().as_posix()}")'
@@ -565,19 +565,57 @@ def xmlparse(self):
 
             self.quantitiesList.append(scalar)
 
-    def getQuantities(self, names=None):  # 3
+    def getQuantities(self, names: Optional[str | list[str]] = None) -> list[dict]:
         """
-        This method returns list of dictionaries. It displays details of quantities such as name, value, changeable, and description, where changeable means  if value for corresponding quantity name is changeable or not. It can be called :
-        usage:
-        >>> getQuantities()
-        >>> getQuantities("Name1")
-        >>> getQuantities(["Name1","Name2"])
+        This method returns list of dictionaries. It displays details of
+        quantities such as name, value, changeable, and description.
+
+        Examples:
+            >>> mod.getQuantities()
+            [
+              {
+                'alias': 'noAlias',
+                'aliasvariable': None,
+                'causality': 'local',
+                'changeable': 'true',
+                'description': None,
+                'max': None,
+                'min': None,
+                'name': 'x',
+                'start': '1.0',
+                'unit': None,
+                'variability': 'continuous',
+              },
+              {
+                'name': 'der(x)',
+                # ...
+              },
+              # ...
+            ]
+
+            >>> getQuantities("y")
+            [{
+              'name': 'y', # ...
+            }]
+
+            >>> getQuantities(["y","x"])
+            [
+              {
+                'name': 'y', # ...
+              },
+              {
+                'name': 'x', # ...
+              }
+            ]
         """
         if names is None:
             return self.quantitiesList
 
         if isinstance(names, str):
-            return [x for x in self.quantitiesList if x["name"] == names]
+            r = [x for x in self.quantitiesList if x["name"] == names]
+            if r == []:
+                raise KeyError(names)
+            return r
 
         if isinstance(names, list):
             return [x for y in names for x in self.quantitiesList if x["name"] == y]
@@ -597,10 +635,10 @@ def getContinuous(self, names=None):  # 4
                 return self.continuouslist
 
             if isinstance(names, str):
-                return [self.continuouslist.get(names, "NotExist")]
+                return [self.continuouslist[names]]
 
             if isinstance(names, list):
-                return [self.continuouslist.get(x, "NotExist") for x in names]
+                return [self.continuouslist[x] for x in names]
         else:
             if names is None:
                 for i in self.continuouslist:
@@ -615,7 +653,7 @@ def getContinuous(self, names=None):  # 4
                 if names in self.continuouslist:
                     value = self.getSolutions(names)
                     self.continuouslist[names] = value[0][-1]
-                    return [self.continuouslist.get(names)]
+                    return [self.continuouslist[names]]
                 else:
                     raise ModelicaSystemError(f"{names} is not continuous")
 
@@ -657,9 +695,9 @@ def getParameters(self, names: Optional[str | list[str]] = None) -> dict[str, st
         if names is None:
             return self.paramlist
         elif isinstance(names, str):
-            return [self.paramlist.get(names, "NotExist")]
+            return [self.paramlist[names]]
         elif isinstance(names, list):
-            return [self.paramlist.get(x, "NotExist") for x in names]
+            return [self.paramlist[x] for x in names]
 
         raise ModelicaSystemError("Unhandled input for getParameters()")
 
@@ -687,15 +725,13 @@ def getInputs(self, names: Optional[str | list[str]] = None) -> dict | list:  #
             [[(0.0, 0.0), (1.0, 1.0)]]
             >>> mod.getInputs(["Name1","Name2"])
             [[(0.0, 0.0), (1.0, 1.0)], None]
-            >>> mod.getInputs("ThisInputDoesNotExist")
-            ['NotExist']
         """
         if names is None:
             return self.inputlist
         elif isinstance(names, str):
-            return [self.inputlist.get(names, "NotExist")]
+            return [self.inputlist[names]]
         elif isinstance(names, list):
-            return [self.inputlist.get(x, "NotExist") for x in names]
+            return [self.inputlist[x] for x in names]
 
         raise ModelicaSystemError("Unhandled input for getInputs()")
 
@@ -725,8 +761,6 @@ def getOutputs(self, names: Optional[str | list[str]] = None):  # 7
             ['-0.4']
             >>> mod.getOutputs(["out1","out2"])
             ['-0.4', '1.2']
-            >>> mod.getOutputs("ThisOutputDoesNotExist")
-            ['NotExist']
 
             After simulate():
             >>> mod.getOutputs()
@@ -740,9 +774,9 @@ def getOutputs(self, names: Optional[str | list[str]] = None):  # 7
             if names is None:
                 return self.outputlist
             elif isinstance(names, str):
-                return [self.outputlist.get(names, "NotExist")]
+                return [self.outputlist[names]]
             else:
-                return [self.outputlist.get(x, "NotExist") for x in names]
+                return [self.outputlist[x] for x in names]
         else:
             if names is None:
                 for i in self.outputlist:
@@ -753,9 +787,9 @@ def getOutputs(self, names: Optional[str | list[str]] = None):  # 7
                 if names in self.outputlist:
                     value = self.getSolutions(names)
                     self.outputlist[names] = value[0][-1]
-                    return [self.outputlist.get(names)]
+                    return [self.outputlist[names]]
                 else:
-                    return names, " is not Output"
+                    raise KeyError(names)
             elif isinstance(names, list):
                 valuelist = []
                 for i in names:
@@ -764,7 +798,7 @@ def getOutputs(self, names: Optional[str | list[str]] = None):  # 7
                         self.outputlist[i] = value[0][-1]
                         valuelist.append(value[0][-1])
                     else:
-                        return i, "is not Output"
+                        raise KeyError(i)
                 return valuelist
 
         raise ModelicaSystemError("Unhandled input for getOutputs()")
@@ -781,9 +815,9 @@ def getSimulationOptions(self, names=None):  # 8
         if names is None:
             return self.simulateOptions
         elif isinstance(names, str):
-            return [self.simulateOptions.get(names, "NotExist")]
+            return [self.simulateOptions[names]]
         elif isinstance(names, list):
-            return [self.simulateOptions.get(x, "NotExist") for x in names]
+            return [self.simulateOptions[x] for x in names]
 
         raise ModelicaSystemError("Unhandled input for getSimulationOptions()")
 
@@ -799,9 +833,9 @@ def getLinearizationOptions(self, names=None):  # 9
         if names is None:
             return self.linearOptions
         elif isinstance(names, str):
-            return [self.linearOptions.get(names, "NotExist")]
+            return [self.linearOptions[names]]
         elif isinstance(names, list):
-            return [self.linearOptions.get(x, "NotExist") for x in names]
+            return [self.linearOptions[x] for x in names]
 
         raise ModelicaSystemError("Unhandled input for getLinearizationOptions()")
 
@@ -815,9 +849,9 @@ def getOptimizationOptions(self, names=None):  # 10
         if names is None:
             return self.optimizeOptions
         elif isinstance(names, str):
-            return [self.optimizeOptions.get(names, "NotExist")]
+            return [self.optimizeOptions[names]]
         elif isinstance(names, list):
-            return [self.optimizeOptions.get(x, "NotExist") for x in names]
+            return [self.optimizeOptions[x] for x in names]
 
         raise ModelicaSystemError("Unhandled input for getOptimizationOptions()")
 
@@ -1236,8 +1270,10 @@ def load_module_from_path(module_name, file_path):
             return module_def
 
         if self.xmlFile is None:
-            raise IOError("Linearization cannot be performed as the model is not build, "
-                          "use ModelicaSystem() to build the model first")
+            raise ModelicaSystemError(
+                "Linearization cannot be performed as the model is not build, "
+                "use ModelicaSystem() to build the model first"
+            )
 
         om_cmd = ModelicaSystemCmd(runpath=self.tempdir, modelname=self.modelName, timeout=timeout)
 

tests/test_ModelicaSystem.py

@@ -43,6 +43,8 @@ def test_setParameters():
         "e": "1.234",
         "g": "321.0",
     }
+    with pytest.raises(KeyError):
+        mod.getParameters("thisParameterDoesNotExist")
 
     # method 2
     mod.setParameters(["e=21.3", "g=0.12"])
@@ -52,6 +54,8 @@ def test_setParameters():
     }
     assert mod.getParameters(["e", "g"]) == ["21.3", "0.12"]
     assert mod.getParameters(["g", "e"]) == ["0.12", "21.3"]
+    with pytest.raises(KeyError):
+        mod.getParameters(["g", "thisParameterDoesNotExist"])
 
 
 def test_setSimulationOptions():
@@ -69,6 +73,8 @@ def test_setSimulationOptions():
     assert isinstance(d, dict)
     assert d["stopTime"] == "1.234"
     assert d["tolerance"] == "1.1e-08"
+    with pytest.raises(KeyError):
+        mod.getSimulationOptions("thisOptionDoesNotExist")
 
     # method 2
     mod.setSimulationOptions(["stopTime=2.1", "tolerance=1.2e-08"])
@@ -125,7 +131,7 @@ def test_getSolutions(model_firstorder):
     assert "x" in sol_names
     assert "der(x)" in sol_names
     with pytest.raises(OMPython.ModelicaSystemError):
-        mod.getSolutions("t")  # variable 't' does not exist
+        mod.getSolutions("thisVariableDoesNotExist")
     assert np.isclose(t[0], 0), "time does not start at 0"
     assert np.isclose(t[-1], stopTime), "time does not end at stopTime"
     x_analytical = x0 * np.exp(a*t)
@@ -262,11 +268,18 @@ def test_getters(tmp_path):
         },
     ]
 
+    with pytest.raises(KeyError):
+        mod.getQuantities("thisQuantityDoesNotExist")
+
     assert mod.getInputs() == {}
+    with pytest.raises(KeyError):
+        mod.getInputs("thisInputDoesNotExist")
     # getOutputs before simulate()
     assert mod.getOutputs() == {'y': '-0.4'}
     assert mod.getOutputs("y") == ["-0.4"]
     assert mod.getOutputs(["y", "y"]) == ["-0.4", "-0.4"]
+    with pytest.raises(KeyError):
+        mod.getOutputs("thisOutputDoesNotExist")
 
     # getContinuous before simulate():
     assert mod.getContinuous() == {
@@ -276,7 +289,8 @@ def test_getters(tmp_path):
     }
     assert mod.getContinuous("y") == ['-0.4']
     assert mod.getContinuous(["y", "x"]) == ['-0.4', '1.0']
-    assert mod.getContinuous("a") == ["NotExist"]  # a is a parameter
+    with pytest.raises(KeyError):
+        mod.getContinuous("a")  # a is a parameter
 
     stopTime = 1.0
     a = -0.5
@@ -293,6 +307,8 @@ def test_getters(tmp_path):
     assert np.isclose(d["y"], dx_analytical, 1e-4)
     assert mod.getOutputs("y") == [d["y"]]
     assert mod.getOutputs(["y", "y"]) == [d["y"], d["y"]]
+    with pytest.raises(KeyError):
+        mod.getOutputs("thisOutputDoesNotExist")
 
     # getContinuous after simulate() should return values at end of simulation:
     with pytest.raises(OMPython.ModelicaSystemError):
@@ -307,6 +323,9 @@ def test_getters(tmp_path):
     assert mod.getContinuous("x") == [d["x"]]
     assert mod.getContinuous(["y", "x"]) == [d["y"], d["x"]]
 
+    with pytest.raises(OMPython.ModelicaSystemError):
+        mod.getContinuous("a")  # a is a parameter
+
     with pytest.raises(OMPython.ModelicaSystemError):
         mod.setSimulationOptions("thisOptionDoesNotExist=3")
 

tests/test_ModelicaSystemCmd.py

@@ -23,7 +23,8 @@ def test_simflags(model_firstorder):
         "noRestart": None,
         "override": {'b': 2}
     })
-    mscmd.args_set(args=mscmd.parse_simflags(simflags="-noEventEmit -noRestart -override=a=1,x=3"))
+    with pytest.deprecated_call():
+        mscmd.args_set(args=mscmd.parse_simflags(simflags="-noEventEmit -noRestart -override=a=1,x=3"))
 
     assert mscmd.get_cmd() == [
         mscmd.get_exe().as_posix(),

tests/test_ZMQ.py

@@ -37,6 +37,7 @@ def test_Simulate(om, model_time_str):
 
 
 def test_execute(om):
-    assert om.execute('"HelloWorld!"') == '"HelloWorld!"\n'
+    with pytest.deprecated_call():
+        assert om.execute('"HelloWorld!"') == '"HelloWorld!"\n'
     assert om.sendExpression('"HelloWorld!"', parsed=False) == '"HelloWorld!"\n'
     assert om.sendExpression('"HelloWorld!"', parsed=True) == 'HelloWorld!'