Merge pull request #548 from avinxshKD/fix/read-backward-compat

fix: make read() backward compatible again

Author: pradeeban

concore.py

@@ -223,25 +223,19 @@ def unchanged():
 def read(port_identifier, name, initstr_val):
     """Read data from a ZMQ port or file-based port.
 
-    Returns:
-        tuple: (data, success_flag) where success_flag is True if real
-            data was received, False if a fallback/default was used.
-            Also sets ``concore.last_read_status`` to one of:
-            SUCCESS, FILE_NOT_FOUND, TIMEOUT, PARSE_ERROR,
-            EMPTY_DATA, RETRIES_EXCEEDED.
-
-    Backward compatibility:
-        Legacy callers that do ``value = concore.read(...)`` will
-        receive a tuple.  They can adapt with::
-
-            result = concore.read(...)
-            if isinstance(result, tuple):
-                value, ok = result
-            else:
-                value, ok = result, True
+    Returns only the data payload for backward compatibility.
+    Use ``read_with_status()`` for explicit ``(data, success_flag)``.
+    """
+    global last_read_status
+    result, _ok = concore_base.read(_mod, port_identifier, name, initstr_val)
+    last_read_status = concore_base.last_read_status
+    return result
+
+
+def read_with_status(port_identifier, name, initstr_val):
+    """Read data and return ``(data, success_flag)``.
 
-        Alternatively, check ``concore.last_read_status`` after the
-        call.
+    Also updates ``concore.last_read_status``.
     """
     global last_read_status
     result = concore_base.read(_mod, port_identifier, name, initstr_val)

concore_base.py

@@ -249,18 +249,11 @@ def read(mod, port_identifier, name, initstr_val):
             SUCCESS, FILE_NOT_FOUND, TIMEOUT, PARSE_ERROR,
             EMPTY_DATA, RETRIES_EXCEEDED.
 
-    Backward compatibility:
-        Legacy callers that do ``value = concore.read(...)`` will
-        receive a tuple.  They can adapt with::
-
-            result = concore.read(...)
-            if isinstance(result, tuple):
-                value, ok = result
-            else:
-                value, ok = result, True
-
-        Alternatively, check ``concore.last_read_status`` after the
-        call.
+    Notes:
+        This low-level helper always returns ``(data, success_flag)``.
+        The wrapper modules expose:
+        - ``read()`` for backward-compatible data-only reads.
+        - ``read_with_status()`` for explicit ``(data, success_flag)``.
     """
     global last_read_status
 

concoredocker.py

@@ -92,6 +92,12 @@ def unchanged():
 # I/O Handling (File + ZMQ)
 # ===================================================================
 def read(port_identifier, name, initstr_val):
+    global last_read_status
+    result, _ok = concore_base.read(_mod, port_identifier, name, initstr_val)
+    last_read_status = concore_base.last_read_status
+    return result
+
+def read_with_status(port_identifier, name, initstr_val):
     global last_read_status
     result = concore_base.read(_mod, port_identifier, name, initstr_val)
     last_read_status = concore_base.last_read_status

tests/test_concore.py

@@ -300,7 +300,7 @@ def recv_json_with_retry(self):
         concore.write("roundtrip_test", "data", original_data)
 
         # Read should return original data (simtime stripped) plus success flag
-        result, ok = concore.read("roundtrip_test", "data", "[]")
+        result, ok = concore.read_with_status("roundtrip_test", "data", "[]")
         assert result == original_data
         assert ok is True
 
@@ -520,7 +520,7 @@ def recv_json_with_retry(self):
             concore.zmq_ports["t_in"] = TimeoutPort()
             concore.simtime = 0
 
-            result, ok = concore.read("t_in", "x", "[0.0]")
+            result, ok = concore.read_with_status("t_in", "x", "[0.0]")
 
             assert result == [0.0]
             assert ok is False

tests/test_concoredocker.py

@@ -136,7 +136,7 @@ def test_reads_and_parses_data(self, temp_dir):
 
         concoredocker.s = ""
         concoredocker.simtime = 0
-        result, ok = concoredocker.read(1, "data", "[0, 0, 0]")
+        result, ok = concoredocker.read_with_status(1, "data", "[0, 0, 0]")
 
         assert result == [100, 200]
         assert ok is True
@@ -156,7 +156,7 @@ def test_returns_default_when_file_missing(self, temp_dir):
 
         concoredocker.s = ""
         concoredocker.simtime = 0
-        result, ok = concoredocker.read(1, "nofile", "[0, 5, 5]")
+        result, ok = concoredocker.read_with_status(1, "nofile", "[0, 5, 5]")
 
         assert result == [5, 5]
         assert ok is False
@@ -204,7 +204,7 @@ def recv_json_with_retry(self):
         concoredocker.zmq_ports["test_zmq"] = DummyPort()
         concoredocker.simtime = 0
 
-        result, ok = concoredocker.read("test_zmq", "data", "[]")
+        result, ok = concoredocker.read_with_status("test_zmq", "data", "[]")
 
         assert result == [4.0, 5.0]
         assert ok is True
@@ -243,7 +243,7 @@ def recv_json_with_retry(self):
 
         original = [1.5, 2.5, 3.5]
         concoredocker.write("roundtrip", "data", original)
-        result, ok = concoredocker.read("roundtrip", "data", "[]")
+        result, ok = concoredocker.read_with_status("roundtrip", "data", "[]")
 
         assert result == original
         assert ok is True
@@ -278,7 +278,7 @@ def recv_json_with_retry(self):
             concoredocker.zmq_ports["t_in"] = TimeoutPort()
             concoredocker.simtime = 0
 
-            result, ok = concoredocker.read("t_in", "x", "[0.0]")
+            result, ok = concoredocker.read_with_status("t_in", "x", "[0.0]")
 
             assert result == [0.0]
             assert ok is False

tests/test_protocol_conformance.py

@@ -116,7 +116,7 @@ def _run_read_file_case(case):
                 ) as f:
                     f.write(case["input"]["file_content"])
 
-            result, ok = concore.read(
+            result, ok = concore.read_with_status(
                 case["input"]["port"],
                 case["input"]["name"],
                 case["input"]["initstr_val"],

tests/test_read_status.py

@@ -1,7 +1,8 @@
 """Tests for read() error signalling (Issue #390).
 
-read() now returns (data, success_flag) and sets
-concore.last_read_status / concore_base.last_read_status.
+read() returns data for backward compatibility.
+read_with_status() returns (data, success_flag).
+Both update concore.last_read_status / concore_base.last_read_status.
 """
 
 import os
@@ -35,7 +36,7 @@ def recv_json_with_retry(self):
 
 
 class TestReadFileSuccess:
-    """read() on a valid file returns (data, True) with SUCCESS status."""
+    """read_with_status() on a valid file returns (data, True)."""
 
     @pytest.fixture(autouse=True)
     def setup(self, temp_dir, monkeypatch):
@@ -53,7 +54,7 @@ def setup(self, temp_dir, monkeypatch):
         monkeypatch.setattr(concore, "inpath", os.path.join(temp_dir, "in"))
 
     def test_returns_data_and_true(self):
-        data, ok = self.concore.read(1, "ym", "[0, 0.0]")
+        data, ok = self.concore.read_with_status(1, "ym", "[0, 0.0]")
         assert ok is True
         assert data == [3.14]
 
@@ -63,7 +64,7 @@ def test_last_read_status_is_success(self):
 
 
 class TestReadFileMissing:
-    """read() on a missing file returns (default, False) with FILE_NOT_FOUND."""
+    """read_with_status() on missing file returns (default, False)."""
 
     @pytest.fixture(autouse=True)
     def setup(self, temp_dir, monkeypatch):
@@ -75,7 +76,7 @@ def setup(self, temp_dir, monkeypatch):
         monkeypatch.setattr(concore, "inpath", os.path.join(temp_dir, "in"))
 
     def test_returns_default_and_false(self):
-        data, ok = self.concore.read(1, "nonexistent", "[0, 0.0]")
+        data, ok = self.concore.read_with_status(1, "nonexistent", "[0, 0.0]")
         assert ok is False
 
     def test_last_read_status_is_file_not_found(self):
@@ -84,7 +85,7 @@ def test_last_read_status_is_file_not_found(self):
 
 
 class TestReadFileParseError:
-    """read() returns (default, False) with PARSE_ERROR on malformed content."""
+    """read_with_status() returns (default, False) on parse errors."""
 
     @pytest.fixture(autouse=True)
     def setup(self, temp_dir, monkeypatch):
@@ -101,7 +102,7 @@ def setup(self, temp_dir, monkeypatch):
         monkeypatch.setattr(concore, "inpath", os.path.join(temp_dir, "in"))
 
     def test_returns_default_and_false(self):
-        data, ok = self.concore.read(1, "ym", "[0, 0.0]")
+        data, ok = self.concore.read_with_status(1, "ym", "[0, 0.0]")
         assert ok is False
 
     def test_last_read_status_is_parse_error(self):
@@ -110,7 +111,7 @@ def test_last_read_status_is_parse_error(self):
 
 
 class TestReadFileTraversalBlocked:
-    """read() rejects traversal names and returns PARSE_ERROR."""
+    """read_with_status() rejects traversal names and returns PARSE_ERROR."""
 
     @pytest.fixture(autouse=True)
     def setup(self, temp_dir, monkeypatch):
@@ -126,7 +127,7 @@ def setup(self, temp_dir, monkeypatch):
             f.write("[10, 3.14]")
 
     def test_returns_default_and_false_on_traversal_name(self):
-        data, ok = self.concore.read(1, "../ym", "[0, 0.0]")
+        data, ok = self.concore.read_with_status(1, "../ym", "[0, 0.0]")
         assert ok is False
         assert data == [0, 0.0]
 
@@ -136,7 +137,7 @@ def test_last_read_status_is_parse_error_for_traversal_name(self):
 
 
 class TestReadFileRetriesExceeded:
-    """read() returns (default, False) with RETRIES_EXCEEDED when file is empty."""
+    """read_with_status() returns (default, False) on retry exhaustion."""
 
     @pytest.fixture(autouse=True)
     def setup(self, temp_dir, monkeypatch):
@@ -154,7 +155,7 @@ def setup(self, temp_dir, monkeypatch):
         monkeypatch.setattr(concore, "inpath", os.path.join(temp_dir, "in"))
 
     def test_returns_default_and_false(self):
-        data, ok = self.concore.read(1, "ym", "[0, 0.0]")
+        data, ok = self.concore.read_with_status(1, "ym", "[0, 0.0]")
         assert ok is False
 
     def test_last_read_status_is_retries_exceeded(self):
@@ -168,7 +169,7 @@ def test_last_read_status_is_retries_exceeded(self):
 
 
 class TestReadZMQSuccess:
-    """Successful ZMQ read returns (data, True)."""
+    """Successful ZMQ read_with_status() returns (data, True)."""
 
     @pytest.fixture(autouse=True)
     def setup(self, monkeypatch):
@@ -185,14 +186,14 @@ def test_zmq_read_returns_data_and_true(self):
         self.concore.zmq_ports["test_port"] = dummy
         self.concore.simtime = 0
 
-        data, ok = self.concore.read("test_port", "ym", "[]")
+        data, ok = self.concore.read_with_status("test_port", "ym", "[]")
         assert ok is True
         assert data == [1.1, 2.2]
         assert self.concore.last_read_status == "SUCCESS"
 
 
 class TestReadZMQTimeout:
-    """ZMQ read that returns None (timeout) yields (default, False)."""
+    """ZMQ read timeout yields (default, False) via read_with_status()."""
 
     @pytest.fixture(autouse=True)
     def setup(self, monkeypatch):
@@ -210,13 +211,13 @@ def test_zmq_timeout_returns_default_and_false(self):
         )
         self.concore.zmq_ports["test_port"] = dummy
 
-        data, ok = self.concore.read("test_port", "ym", "[]")
+        data, ok = self.concore.read_with_status("test_port", "ym", "[]")
         assert ok is False
         assert self.concore.last_read_status == "TIMEOUT"
 
 
 class TestReadZMQError:
-    """ZMQ read that raises ZMQError yields (default, False)."""
+    """ZMQ read ZMQError yields (default, False) via read_with_status()."""
 
     @pytest.fixture(autouse=True)
     def setup(self, monkeypatch):
@@ -234,7 +235,7 @@ def test_zmq_error_returns_default_and_false(self):
         dummy = DummyZMQPort(raise_on_recv=zmq.error.ZMQError("test error"))
         self.concore.zmq_ports["test_port"] = dummy
 
-        data, ok = self.concore.read("test_port", "ym", "[]")
+        data, ok = self.concore.read_with_status("test_port", "ym", "[]")
         assert ok is False
         assert self.concore.last_read_status == "TIMEOUT"
 
@@ -245,7 +246,7 @@ def test_zmq_error_returns_default_and_false(self):
 
 
 class TestReadBackwardCompatibility:
-    """Legacy callers can use isinstance check on the result."""
+    """Legacy callers get plain data from read()."""
 
     @pytest.fixture(autouse=True)
     def setup(self, temp_dir, monkeypatch):
@@ -261,22 +262,12 @@ def setup(self, temp_dir, monkeypatch):
 
         monkeypatch.setattr(concore, "inpath", os.path.join(temp_dir, "in"))
 
-    def test_legacy_unpack_pattern(self):
-        """The recommended migration pattern works correctly."""
-        result = self.concore.read(1, "ym", "[0, 0.0]")
-
-        if isinstance(result, tuple):
-            value, ok = result
-        else:
-            value = result
-            ok = True
-
+    def test_read_returns_data_only(self):
+        value = self.concore.read(1, "ym", "[0, 0.0]")
         assert value == [42.0]
-        assert ok is True
 
-    def test_tuple_unpack(self):
-        """New-style callers can unpack directly."""
-        value, ok = self.concore.read(1, "ym", "[0, 0.0]")
+    def test_read_with_status_returns_tuple(self):
+        value, ok = self.concore.read_with_status(1, "ym", "[0, 0.0]")
         assert value == [42.0]
         assert ok is True