Merge pull request #117 from jmbhughes/main

Implements loading variable length packets from CSV

ccsdspy/packet_types.py

@@ -42,8 +42,8 @@ def from_file(cls, file):
         file : str
            Path to file on the local file system that defines the packet fields.
            Currently only supports csv files.
-           See :download:`simple_csv_3col.csv <../../ccsdspy/tests/data/packet_def/simple_csv_3col.csv>`  # noqa: E501
-           and :download:`simple_csv_4col.csv <../../ccsdspy/tests/data/packet_def/simple_csv_4col.csv>`  # noqa: E501
+           See :download:`basic_csv_3col.csv <../../ccsdspy/tests/data/packet_def/basic_csv_3col.csv>`  # noqa: E501
+           and :download:`extended_csv_4col.csv <../../ccsdspy/tests/data/packet_def/extended_csv_4col.csv>`  # noqa: E501
 
         Returns
         -------
@@ -524,6 +524,34 @@ def _prepend_primary_header_fields(existing_fields):
     return return_fields
 
 
+def _parse_csv_array_shape(data_type_str):
+    """Parse a data type string from a CSV to determine the array shape.
+
+    Parameters
+    ----------
+    data_type_str : str
+        Full string specifying the data type, e.g. `uint(1, 2)`
+
+    Returns
+    -------
+    array_shape : str, int, tuple of int
+       Parsed array shape to be used in loading CSV.
+    """
+    array_shape_str = data_type_str[data_type_str.find("(") + 1 : data_type_str.find(")")]
+    if array_shape_str == "expand":
+        array_shape = "expand"
+    elif "," in array_shape_str:
+        try:
+            array_shape = tuple(map(int, array_shape_str.split(", ")))
+        except ValueError:
+            raise ValueError(
+                "Array shape must be `expand`, the name of another field, or a tuple of ints."
+            )
+    else:  # string is either another field for reference or a single integer for a one dimensional array shape
+        array_shape = int(array_shape_str) if array_shape_str.isnumeric() else array_shape_str
+    return array_shape
+
+
 def _get_fields_csv_file(csv_file):
     """Parse a simple comma-delimited file that defines a packet.
 
@@ -554,19 +582,16 @@ def _get_fields_csv_file(csv_file):
             raise ValueError(f"Minimum required columns are {req_columns}.")
 
         for row in reader:  # skip the header row
-            if "bit_offset" not in headers:  # 3 col csv file
+            if "bit_offset" not in headers:  # basic 3 col csv file
                 if (row["data_type"].count("(") == 1) and (row["data_type"].count(")") == 1):
                     data_type = row["data_type"].split("(")[0]
-                    array_shape_str = row["data_type"][
-                        row["data_type"].find("(") + 1 : row["data_type"].find(")")
-                    ]
-                    array_shape = tuple(map(int, array_shape_str.split(", ")))
+                    array_shape = _parse_csv_array_shape(row["data_type"])
                     fields.append(
                         PacketArray(
                             name=row["name"],
                             data_type=data_type,
                             bit_length=int(row["bit_length"]),
-                            array_shape=(array_shape),
+                            array_shape=array_shape,
                         )
                     )
                 else:
@@ -577,14 +602,11 @@ def _get_fields_csv_file(csv_file):
                             bit_length=int(row["bit_length"]),
                         )
                     )
-            if "bit_offset" in headers:  # 4 col csv file provides bit offsets
+            if "bit_offset" in headers:  # extended 4 col csv file provides bit offsets
                 # TODO: Check the consistency of bit_offsets versus previous bit_lengths
                 if (row["data_type"].count("(") == 1) and (row["data_type"].count(")") == 1):
                     data_type = row["data_type"].split("(")[0]
-                    array_shape_str = row["data_type"][
-                        row["data_type"].find("(") + 1 : row["data_type"].find(")")
-                    ]
-                    array_shape = tuple(map(int, array_shape_str.split(", ")))
+                    array_shape = _parse_csv_array_shape(row["data_type"])
                     fields.append(
                         PacketArray(
                             name=row["name"],

ccsdspy/tests/data/packet_def/basic_csv_3col_with_all.csv

@@ -0,0 +1,6 @@
+name, data_type, bit_length
+SHCOARSE, uint, 32
+SHFINE, "uint(expand)", 20
+OPMODE, "uint(SHFINE)", 3
+SPACER, fill, 1
+VOLTAGE, "int(12, 24)", 8

ccsdspy/tests/data/packet_def/basic_csv_3col_with_expand.csv

@@ -0,0 +1,6 @@
+name, data_type, bit_length
+SHCOARSE, uint, 32
+SHFINE, uint, 20
+OPMODE, uint, 3
+SPACER, fill, 1
+VOLTAGE, "uint(expand)", 8

ccsdspy/tests/data/packet_def/basic_csv_3col_with_reference.csv

@@ -0,0 +1,6 @@
+name, data_type, bit_length
+SHCOARSE, uint, 32
+SHFINE, uint, 20
+OPMODE, uint, 3
+SPACER, fill, 1
+VOLTAGE, "uint(OPMODE)", 8

ccsdspy/tests/test_packet_types.py

@@ -11,14 +11,17 @@
 
 from .. import FixedLength, VariableLength, PacketField, PacketArray
 from ..constants import BITS_PER_BYTE
-from ..packet_types import _get_fields_csv_file
+from ..packet_types import _get_fields_csv_file, _parse_csv_array_shape
 
 dir_path = os.path.dirname(os.path.realpath(__file__))
 packet_def_dir = os.path.join(dir_path, "data", "packet_def")
-csv_file_4col = os.path.join(packet_def_dir, "simple_csv_4col.csv")
-csv_file_3col = os.path.join(packet_def_dir, "simple_csv_3col.csv")
-csv_file_4col_with_array = os.path.join(packet_def_dir, "simple_csv_4col_with_array.csv")
-csv_file_3col_with_array = os.path.join(packet_def_dir, "simple_csv_3col_with_array.csv")
+csv_file_4col = os.path.join(packet_def_dir, "extended_csv_4col.csv")
+csv_file_3col = os.path.join(packet_def_dir, "basic_csv_3col.csv")
+csv_file_4col_with_array = os.path.join(packet_def_dir, "extended_csv_4col_with_array.csv")
+csv_file_3col_with_array = os.path.join(packet_def_dir, "basic_csv_3col_with_array.csv")
+csv_file_3col_with_expand = os.path.join(packet_def_dir, "basic_csv_3col_with_expand.csv")
+csv_file_3col_with_reference = os.path.join(packet_def_dir, "basic_csv_3col_with_reference.csv")
+csv_file_3col_with_all = os.path.join(packet_def_dir, "basic_csv_3col_with_all.csv")
 
 hs_packet_dir = os.path.join(dir_path, "data", "hs")
 random_binary_file = os.path.join(
@@ -102,6 +105,31 @@ def test_FixedLength_from_file_not_supported(filename):
         FixedLength.from_file(filename)
 
 
+@pytest.mark.parametrize(
+    "shape_str, expected_value",
+    [
+        ("uint(4)", 4),
+        ("uint(1, 2)", (1, 2)),
+        ("uint(expand)", "expand"),
+        ("uint(OPMODE)", "OPMODE"),
+    ],
+)
+def test_parse_csv_array_shape(shape_str, expected_value):
+    assert _parse_csv_array_shape(shape_str) == expected_value
+
+
+def test_parse_csv_array_shape_fails_on_invalid_shape_str():
+    with pytest.raises(ValueError):
+        _parse_csv_array_shape("uint(4, FIELD)")
+
+
+def test_VariableLength_from_file():
+    """Test that from_file returns a VariableLength instance"""
+    assert isinstance(VariableLength.from_file(csv_file_3col_with_expand), VariableLength)
+    assert isinstance(VariableLength.from_file(csv_file_3col_with_reference), VariableLength)
+    assert isinstance(VariableLength.from_file(csv_file_3col_with_all), VariableLength)
+
+
 @pytest.mark.parametrize(
     "cls,numpy_dtype,ccsdspy_data_type,ccsdspy_bit_length,array_order,include_bit_offset",
     [

docs/user-guide/loadfile.rst

@@ -9,15 +9,22 @@ Overview
 
 :ref:`fixed` can be loaded from a CSV (comma separated value) file.
 This is an alternative method for defining packet layouts which may be desirable to some users,
-and is currently undergoing development. The syntax for loading from a CSV file is:
+and is currently undergoing development. The syntax for loading a `~ccsdspy.FixedLength` packet from a CSV file is:
 
 .. code-block:: python
 
    import ccsdspy
    pkt = ccsdspy.FixedLength.from_file('packet_definition.csv')
 
+The syntax is the same for `~ccdspy.VariableLength` packets:
+
+.. code-block:: python
+
+   import ccsdspy
+   pkt = ccsdspy.VariableLength.from_file('packet_definition.csv')
+
 The only requirement is that the CSV is structured as either the :ref:`threecolumn`
-or :ref:`fourcolumn`. At the moment, :ref:`variable` cannot be loaded from a CSV file.
+or :ref:`fourcolumn`.
 
 .. contents::
    :depth: 2
@@ -34,7 +41,7 @@ calculates the bit offsets assuming that the packet order is correct. See the :r
 for more flexibility.
 
 .. csv-table:: Basic Layout CSV
-   :file: ../../ccsdspy/tests/data/packet_def/simple_csv_3col.csv
+   :file: ../../ccsdspy/tests/data/packet_def/basic_csv_3col.csv
    :widths: 30, 30, 30
    :header-rows: 1
 
@@ -43,12 +50,19 @@ with varying names, data types, and bit lengths. To create a `~ccsdspy.PacketArr
 both the type and array shape.
 
 .. csv-table:: Basic Layout CSV with `~ccsdspy.PacketArray`
-   :file: ../../ccsdspy/tests/data/packet_def/simple_csv_3col_with_array.csv
+   :file: ../../ccsdspy/tests/data/packet_def/basic_csv_3col_with_array.csv
    :widths: 30, 30, 30
    :header-rows: 1
 
 In the example above, `VOLTAGE` would instead be a `~ccsdspy.PacketArray` of type `int` with shape `(12, 24)`.
 
+For :ref:`variable`, the array shape string can be specified either as `expand` or as the name of another field.
+
+.. csv-table:: Basic Layout CSV with `~ccsdspy.PacketArray` for Variable Length Packets
+   :file: ../../ccsdspy/tests/data/packet_def/basic_csv_3col_with_all.csv
+   :widths: 30, 30, 30
+   :header-rows: 1
+
 .. _fourcolumn:
 
 Extended Layout (Four Columns)
@@ -61,7 +75,7 @@ of automatically calculated. Due to this, some packet fields can be skipped
 since the bit offset indicates exactly where the packet begins.
 
 .. csv-table:: Extended Layout CSV
-   :file: ../../ccsdspy/tests/data/packet_def/simple_csv_4col.csv
+   :file: ../../ccsdspy/tests/data/packet_def/extended_csv_4col.csv
    :widths: 30, 30, 30, 30
    :header-rows: 1
 
@@ -70,17 +84,19 @@ with varying names, data types, and bit lengths. To create a `~ccsdspy.PacketArr
 both the type and array shape.
 
 .. csv-table:: Extended Layout CSV with `~ccsdspy.PacketArray`
-   :file: ../../ccsdspy/tests/data/packet_def/simple_csv_4col_with_array.csv
+   :file: ../../ccsdspy/tests/data/packet_def/extended_csv_4col_with_array.csv
    :widths: 30, 30, 30, 30
    :header-rows: 1
 
 In the example above, `SHSCOARSE` would instead be a `~ccsdspy.PacketArray` of type `uint` with shape `(4)`.
 
+.. note::
+    :ref:`variable` are not supported in the extended layout since `bit_offset` cannot be specified for variable length packets.
+
 Limitations of the CSV format
 =============================
 
 The CSV format is in development and is currently limited. The limitations are:
 
 * the byte order cannot be defined in the CSV.
-* the array order and byte order cannot be defined in the CSV.
-* :ref:`variable` cannot currently be loaded from a CSV file.
+* the array order cannot be defined in the CSV.

docs/user-guide/variablelength.rst

@@ -29,6 +29,9 @@ The result will be a dictionary with the names as the keys.
 The values are arrays with the `~ccsdspy.PacketArray` field providing arrays with variable sizes.
 It is also possible to get access to the packet primary header. See :ref:`getting-header`.
 
+.. warning::
+    `bit_offset` cannot be specified for variable length packets. Instead, the packet definition must define all packets, and the bit offsets are calculated automatically.
+
 .. contents::
    :depth: 2
 

pyproject.toml

@@ -33,7 +33,7 @@ dependencies = [
 [project.optional-dependencies]
 dev = [
   'coverage>=6.5.0',
-  'pytest>=7.1.3',
+  'pytest>=7.1.3, <8.1.0',
   'pytest-astropy',
   'pytest-cov',
   'black==22.10.0',
@@ -61,7 +61,7 @@ write_to = "ccsdspy/_version.py"
 minversion = "6.0"
 testpaths = ["ccsdspy/tests", "docs"]
 norecursedirs = ["build", "docs/_build", "docs/generated", "*.egg-info", "attic"]
-doctest_plus = "enabled"
+#doctest_plus = "enabled"
 text_file_format = "rst"
 addopts = "--doctest-modules"
 collect_ignore_glob= ['_*.py']