Code review changes

- Rewrites the note on waveform of strings
- Refactors _require_waveform
- Removes unused DTYPE and add more tests for mixed lists

Author: AlexanderWells-diamond

docs/reference/api.rst

@@ -343,9 +343,9 @@ All functions return a wrapped `ProcessDeviceSupportIn` or
     if given, or defaults to ``'FLOAT'``.
 
     .. note::
-        When storing arrays of strings, it is possible to store Unicode characters.
-        However, as EPICS has no Unicode support the resultant values will be stored
-        as byte strings. Care must be taken when encoding/decoding the values.
+        Storing arrays of strings differs from other values. String arrays will always
+        be assumed to be encoded as Unicode strings, and will be returned to the user
+        as a Python list rather than a Numpy array.
 
 
 The following functions generates specialised records.

softioc/device.py

@@ -367,18 +367,18 @@ def to_epics_str_array(value):
 
 
 def _require_waveform(value, dtype):
-    if isinstance(value, bytes):
-        # Special case hack for byte arrays.  Surprisingly tricky:
-        value = numpy.frombuffer(value, dtype = numpy.uint8)
-
     if dtype and dtype.char == 'S':
         return to_epics_str_array(value)
-
-    value = numpy.require(value, dtype = dtype)
-    if value.shape == ():
-        value.shape = (1,)
-    assert value.ndim == 1, 'Can\'t write multidimensional arrays'
-    return value
+    else:
+        if isinstance(value, bytes):
+            # Special case hack for byte arrays.  Surprisingly tricky:
+            value = numpy.frombuffer(value, dtype = numpy.uint8)
+
+        value = numpy.require(value, dtype = dtype)
+        if value.shape == ():
+            value.shape = (1,)
+        assert value.ndim == 1, 'Can\'t write multidimensional arrays'
+        return value
 
 
 class WaveformBase(ProcessDeviceSupportCore):

tests/test_record_values.py

@@ -31,9 +31,6 @@
     "might think to put into a record that can theoretically hold a huge " \
     "string and so lets test it and prove that shall we?"
 
-# The numpy dtype of all arrays of strings
-NUMPY_DTYPE_STRING = "S40"
-
 
 def record_func_names(fixture_value):
     """Provide a nice name for the record_func fixture"""
@@ -234,6 +231,34 @@ def record_values_names(fixture_value):
         ["123abc", "456def", "7890ghi"],
         list,
     ),
+    (
+        "wIn_mixed_array_1",
+        builder.WaveformIn,
+        ["123abc", 456, "7890ghi"],
+        ["123abc", "456", "7890ghi"],
+        list,
+    ),
+    (
+        "wOut_mixed_array_1",
+        builder.WaveformOut,
+        ["123abc", 456, "7890ghi"],
+        ["123abc", "456", "7890ghi"],
+        list,
+    ),
+    (
+        "wIn_mixed_array_2",
+        builder.WaveformIn,
+        [123, 456, "7890ghi"],
+        ["123", "456", "7890ghi"],
+        list,
+    ),
+    (
+        "wOut_mixed_array_2",
+        builder.WaveformOut,
+        [123, 456, "7890ghi"],
+        ["123", "456", "7890ghi"],
+        list,
+    ),
     (
         "longStringIn_str",
         builder.longStringIn,
@@ -435,7 +460,7 @@ def is_valid(configuration):
 
         if creation_func in (builder.WaveformIn, builder.WaveformOut):
             if isinstance(initial_value, list) and \
-                    all(isinstance(val, (str, bytes)) for val in initial_value):
+                    any(isinstance(val, (str, bytes)) for val in initial_value):
                 if set_enum is not SetValueEnum.INITIAL_VALUE:
                     print(f"Removing {configuration}")
                     return False
@@ -580,7 +605,7 @@ def test_value_pre_init_set(self, record_values):
         if (
             creation_func in [builder.WaveformIn, builder.WaveformOut] and
             isinstance(initial_value, list) and
-            all(isinstance(s, (str, bytes)) for s in initial_value)
+            any(isinstance(s, (str, bytes)) for s in initial_value)
         ):
             pytest.skip("Cannot .set() a list of strings to a waveform, must"
                         "initially specify using initial_value or FTVL")
@@ -974,7 +999,7 @@ def test_waveform_rejects_overlong_values(self):
 
     def test_waveform_rejects_late_strings(self):
         """Test that a waveform won't allow a list of strings to be assigned
-        if no list was given in initial waveform construction"""
+        if no string list was given in initial waveform construction"""
         w_in = builder.WaveformIn("W_IN", length=10)
         w_out = builder.WaveformOut("W_OUT", length=10)
 
@@ -994,11 +1019,13 @@ def test_waveform_rejects_long_array_of_strings(self):
             initial_value=["123abc", "456def", "7890ghi"]
         )
 
+        # Test putting too many elements
         with pytest.raises(AssertionError):
             w_in.set(["1", "2", "3", "4"])
         with pytest.raises(AssertionError):
             w_out.set(["1", "2", "3", "4"])
 
+        # Test putting too long a string
         with pytest.raises(ValueError):
             w_in.set([VERY_LONG_STRING])
         with pytest.raises(ValueError):