Minor fixes and improvements (#15)

* Make error handling and output pipe changes

* Test changes in previous commit

* Correct documentation

* Update changelog and version

* - Fix range
- Improve start and stop of MAD-NG

* Remove closing modifications

* Fix issue of pipe closing prematurely (after 10 seconds).
Now wait for pipe to be ready to be read indefinitely

---------

Co-authored-by: jgray-19 <[email protected]>

Author: jgray-19

src/pymadng/madp_object.py

@@ -2,7 +2,7 @@
 
 import platform
 from pathlib import Path
-from typing import Any, TYPE_CHECKING, TextIO # To make stuff look nicer
+from typing import Any, TYPE_CHECKING, TextIO  # To make stuff look nicer
 
 import numpy as np  # For arrays  (Works well with multiprocessing and mmap)
 
@@ -31,8 +31,9 @@
 
 bin_path = Path(__file__).parent.resolve() / "bin"
 
+
 # --------------------- Overload recv_ref functions ---------------------- #
-# Override the type of reference created by python 
+# Override the type of reference created by python
 # (so madp_pymad can be run independently, these objects create pythonic objects)
 def recv_ref(self: mad_process) -> high_level_mad_ref:
     return high_level_mad_ref(self.varname, self)
@@ -200,25 +201,25 @@ def send(self, data: str | int | float | np.ndarray | bool | list) -> MAD:
         """
         self.__process.send(data)
         return self
-    
+
     def protected_send(self, string: str) -> MAD:
-        """Send a string to MAD-NG, but if any of the command errors, python will be notified. 
+        """Send a string to MAD-NG, but if any of the command errors, python will be notified.
         Then, once python receives the fact that the command errored, it will raise an error.
 
-        For example, if you send ``mad.send("a = 1/'a'")``, MAD-NG will error, and python will just continue. 
-        But if you send ``mad.protected_send("a = 1/'a'").recv()``, python will raise an error. 
+        For example, if you send ``mad.send("a = 1/'a'")``, MAD-NG will error, and python will just continue.
+        But if you send ``mad.protected_send("a = 1/'a'").recv()``, python will raise an error.
         Note, if the ``recv`` is not called, the error will not be raised.
-        
+
         Args:
           string (str): The string to send to MAD-NG. This must be a string to be evaluated in MAD-NG. Not a string to be sent to MAD-NG.
 
         Returns:
-          self (the instance of the mad object)       
+          self (the instance of the mad object)
         """
         assert isinstance(string, str), "The input to protected_send must be a string"
         self.__process.protected_send(string)
         return self
-    
+
     def psend(self, string: str) -> MAD:
         """See :meth:`protected_send`"""
         return self.protected_send(string)
@@ -441,7 +442,7 @@ def __dir__(self) -> Iterable[str]:
         pyObjs.extend(dir(self.recv_vars("_G")))
         return pyObjs
 
-    def globals(self) ->  list[str]:
+    def globals(self) -> list[str]:
         """Retreive the list of names of variables in the environment of MAD
 
         Returns:
@@ -466,7 +467,6 @@ def __enter__(self):
         return self
 
     def __exit__(self, exc_type, exc_value, tb):
-        # Close the process, as the with statement is finished
         self.__process.close()
 
     # ---------------------------------------------------------------------------------------------------#

src/pymadng/madp_pymad.py

@@ -8,6 +8,7 @@
 import sys
 from pathlib import Path
 from typing import Any, TYPE_CHECKING, TextIO
+from contextlib import suppress
 
 import numpy as np
 
@@ -16,6 +17,7 @@
 
 __all__ = ["mad_process"]
 
+# TODO: look at cpymad for the suppression of the error messages at exit - copy? (jgray 2024)
 
 def is_private(varname):
     assert isinstance(varname, str), "Variable name to receive must be a string"
@@ -68,7 +70,8 @@ def __init__(
             stderr = sys.stderr.fileno()
 
         # Create a chunk of code to start the process
-        startupChunk = f"MAD.pymad '{py_name}' {{_dbg = {str(bool(debug)).lower()}}} :__ini({mad_write})"
+        lua_debug_flag = "true" if debug else "false"
+        startupChunk = f"MAD.pymad '{py_name}' {{_dbg = {lua_debug_flag}}} :__ini({mad_write})"
         original_sigint_handler = signal.getsignal(signal.SIGINT)
 
         def delete_process(sig, frame):
@@ -110,15 +113,19 @@ def delete_process(sig, frame):
         self.send("io.stdout:setvbuf('line')")
 
         # Check if MAD started successfully
-        self.send(f"{self.py_name}:send(1)")
+        self.send(f"{self.py_name}:send('started')")
         startup_status_checker = select.select(
             [self.mad_read_stream], [], [], 10
         )  # May not work on windows
 
         # Check if MAD started successfully using select
-        if not startup_status_checker[0] or self.recv() != 1:  # Need to check number?
+        mad_rtrn = self.recv()
+        if not startup_status_checker[0] or mad_rtrn != 'started':
             self.close()
-            raise OSError(f"Unsuccessful starting of {mad_path} process")
+            if mad_rtrn == 'started':
+                raise OSError(f"Could not establish communication with {mad_path} process")
+            else:
+                raise OSError(f"Could not start {mad_path} process, received: {mad_rtrn}")
 
         # Set the error handler to be on by default
         if raise_on_madng_error:
@@ -230,7 +237,7 @@ def close(self) -> None:
         """Close the pipes and wait for the process to finish"""
         if self.process.poll() is None:  # If process is still running
             self.send(f"{self.py_name}:__fin()")  # Tell the mad side to finish
-            open_pipe = select.select([self.mad_read_stream], [], [], 10)
+            open_pipe = select.select([self.mad_read_stream], [], [])
             if open_pipe[0]:
                 # Wait for the mad side to finish (variable name in case of errors that need to be caught elsewhere)
                 close_msg = self.recv("closing")
@@ -241,10 +248,8 @@ def close(self) -> None:
             self.process.terminate()  # Terminate the process on the python side
 
         # Close the debug file if it exists
-        try:
+        with suppress(AttributeError):
             self.stdout_file.close()
-        except AttributeError:
-            pass
 
         # Close the pipes
         if not self.mad_read_stream.closed:

tests/comm_test.py

@@ -223,7 +223,7 @@ class TestRngs(unittest.TestCase):
     def test_recv(self):
         with MAD() as mad:
             mad.send("""
-            irng = 3..11..2
+            irng = MAD.range(3, 11, 2)
             rng = MAD.nrange(3.5, 21.4, 12)
             lrng = MAD.nlogrange(1, 20, 20)
             py:send(irng)

tests/inputs/example.log

@@ -1,9 +1,10 @@
 ***pymad.recv: binary data 4 bytes
 ***pymad.recv: [io.stdout:setvbuf('line')] 25 bytes
 ***pymad.recv: binary data 4 bytes
-***pymad.recv: [py:send(1)] 10 bytes
-***pymad.send: [int_] 4 bytes
+***pymad.recv: [py:send('started')] 18 bytes
+***pymad.send: [str_] 4 bytes
 ***pymad.send: binary data 4 bytes
+***pymad.send: [started] 7 bytes
 ***pymad.recv: binary data 4 bytes
 ***pymad.recv: [element = MAD.element
 sequence = MAD.sequence
@@ -26,7 +27,7 @@ match = MAD.match
 ***pymad.recv: [py:__err(true):send(MAD['env']['version']):__err(false)] 55 bytes
 ***pymad.send: [str_] 4 bytes
 ***pymad.send: binary data 4 bytes
-***pymad.send: [1.0.0] 5 bytes
+***pymad.send: [1.1.0] 5 bytes
 ***pymad.recv: binary data 4 bytes
 ***pymad.recv: [
 function __mklast__ (a, b, ...)

tests/obj_test.py

@@ -373,13 +373,13 @@ def generalDataFrame(self, headers, DataFrame):
 	boolean  = true,
 	list     = {1, 2, 3, 4, 5},
 	table    = {1, 2, ["key"] = "value"},
-	range    = 1..11,
+	range    = MAD.range(1, 11),
 }
-	+ {"a", 1.1, 1, 1 + 2i, true , {1, 2 }, {1 , 2 , ["3" ] = 3 }, 1..11,}
-	+ {"b", 2.2, 2, 2 + 3i, false, {3, 4 }, {4 , 5 , ["6" ] = 6 }, 2..12,}
-	+ {"c", 3.3, 3, 3 + 4i, true , {5, 6 }, {7 , 8 , ["9" ] = 9 }, 3..13,}
-	+ {"d", 4.4, 4, 4 + 5i, false, {7, 8 }, {10, 11, ["12"] = 12}, 4..14,}
-	+ {"e", 5.5, 5, 5 + 6i, true , {9, 10}, {13, 14, ["15"] = 15}, 5..15,}
+	+ {"a", 1.1, 1, 1 + 2i, true , {1, 2 }, {1 , 2 , ["3" ] = 3 }, MAD.range(1, 11),}
+	+ {"b", 2.2, 2, 2 + 3i, false, {3, 4 }, {4 , 5 , ["6" ] = 6 }, MAD.range(2, 12),}
+	+ {"c", 3.3, 3, 3 + 4i, true , {5, 6 }, {7 , 8 , ["9" ] = 9 }, MAD.range(3, 13),}
+	+ {"d", 4.4, 4, 4 + 5i, false, {7, 8 }, {10, 11, ["12"] = 12}, MAD.range(4, 14),}
+	+ {"e", 5.5, 5, 5 + 6i, true , {9, 10}, {13, 14, ["15"] = 15}, MAD.range(5, 15),}
 
 test:addcol("generator", \\ri, m -> m:getcol("number")[ri] + 1i * m:getcol("number")[ri])
 test:write("test")