Repository: ogawa-ros/necstdb

Issue:
Data type "|S12" does not unpacked when opening db specifying  
Below works well
```python
>>>db = necstdb.opendb(data_path)
>>>obsmode = db.open_table("obsmode").read(astype="array")
```

However, 

```python
>>>db = necstdb.opendb(data_path)
>>>obsmode = db.open_table("obsmode").read(astype="df")

>>>print(obsmode["obs_mode"])
b' \x00\x00'
```

pandas is very useful when manipulating time x 1-dimensional data
Could you please fix the issue?@KaoruNishikawa

Discussion:

Patch:
diff --git a/necstdb/necstdb.py b/necstdb/necstdb.py
index 05c1bb3..9715f1e 100644
--- a/necstdb/necstdb.py
+++ b/necstdb/necstdb.py
@@ -7,20 +7,20 @@ timestamp), various kinds of weather data (temperature + humidity + wind speed +
 direction + ... + timestamp), etc.
 """
 
-
-from typing import Union, List, Tuple, Dict, Any
-import re
-import os
+import json
 import mmap
-import struct
+import os
 import pathlib
-import json
+import re
+import struct
 import tarfile
+from typing import Any, Dict, List, Tuple, Union
 
 import numpy
 import pandas
 
 from . import utils
+from .recover import recover
 
 
 def duplicate_rename(path: pathlib.Path, _i: int = 0) -> pathlib.Path:
@@ -423,9 +423,12 @@ class table:
 
             for col in cols:
                 size = struct.calcsize(col["format"])
+                if "x" in col["format"]:  # Pad field
+                    offset += col["size"]
+                    continue
                 dat = struct.unpack(col["format"], data[offset : offset + size])
                 if len(dat) == 1:
-                    dat = dat[0]
+                    (dat,) = dat
 
                 dict_[col["key"]] = dat
                 offset += col["size"]
@@ -448,18 +451,28 @@ class table:
         formats = [col["format"] for col in cols]
 
         def parse_dtype(format_character: str) -> str:
+            def str_format(length: Union[str, int], count: Union[str, int]):
+                count = count if int(count) > 1 else ""
+                return f"{count}S{length}"
+
             format_character = re.sub(
                 r"^([\d+s]+)$",
-                lambda m: f"{m.group(1).count('s')}S{m.group(1).split('s')[0]}",
+                lambda m: str_format(m.group(1).split("s")[0], m.group(1).count("s")),
                 format_character,
             )
+
+            format_character = format_character.replace("x", "V")
             return self.endian + format_character
 
         np_formats = [parse_dtype(col["format"]) for col in cols]
         keys = [col["key"] for col in cols]
         offsets = utils.get_struct_indices(formats, self.endian)[:-1]
+
+        pad = ["x" in col["format"] for col in cols]
+        data_field = [k for k, p in zip(keys, pad) if not p]
+
         dtype = numpy.dtype({"names": keys, "formats": np_formats, "offsets": offsets})
-        return numpy.frombuffer(data, dtype=dtype)
+        return numpy.frombuffer(data, dtype=dtype)[data_field]
 
     @property
     def recovered(self) -> "table":
@@ -481,11 +494,7 @@ class table:
         such as 1e-308)
 
         """
-        self.endian = ""
-        self.open(self._name, self._mode)
-        for dat in self.header["data"]:
-            dat["format"] = dat["format"].replace("i", "?")
-        return self
+        return recover(self)
 
 
 def opendb(path: os.PathLike, mode: str = "r") -> "necstdb":
diff --git a/necstdb/recover.py b/necstdb/recover.py
new file mode 100644
index 0000000..8cd2671
--- /dev/null
+++ b/necstdb/recover.py
@@ -0,0 +1,52 @@
+from typing import TYPE_CHECKING
+
+import numpy
+
+if TYPE_CHECKING:
+    from .necstdb import table
+
+
+def recover(t: "table") -> "table":
+    fmt = "".join([d["format"] for d in t.header["data"]])
+
+    if (t.endian == "<") and ("i" in fmt):
+        t.endian = ""
+        t.open(t._name, t._mode)
+        for dat in t.header["data"]:
+            dat["format"] = dat["format"].replace("i", "?")
+
+    if "s" in fmt:
+        modified_header_data = []
+        for dat in t.header["data"]:
+            if "s" not in dat["format"]:
+                modified_header_data.append(dat)
+            else:
+                dat_numpy = t.read(astype="sa")
+                this_field = dat_numpy[dat["key"]]
+                lengths = numpy.unique([len(d) for d in this_field])
+                if len(lengths) > 1:  # Not uniform length
+                    modified_header_data.append(dat)
+                else:
+                    (length,) = lengths
+                    specified = int(dat["format"].rstrip("s"))
+                    if length == specified:
+                        modified_header_data.append(dat)
+                    else:
+                        diff = specified - length
+                        modified_header_data.append(
+                            {
+                                "key": dat["key"],
+                                "format": f"{length}s",
+                                "size": length,
+                            }
+                        )
+                        modified_header_data.append(
+                            {
+                                "key": f"_{dat['key']}_pad",
+                                "format": f"{diff}x",
+                                "size": diff,
+                            }
+                        )
+        t.header["data"] = modified_header_data
+
+    return t
diff --git a/poetry.lock b/poetry.lock
index 147b715..6c0b1f9 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -23,10 +23,10 @@ optional = false
 python-versions = ">=3.5"
 
 [package.extras]
-dev = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "mypy (>=0.900,!=0.940)", "pytest-mypy-plugins", "zope-interface", "furo", "sphinx", "sphinx-notfound-page", "pre-commit", "cloudpickle"]
-docs = ["furo", "sphinx", "zope-interface", "sphinx-notfound-page"]
-tests = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "mypy (>=0.900,!=0.940)", "pytest-mypy-plugins", "zope-interface", "cloudpickle"]
-tests_no_zope = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "mypy (>=0.900,!=0.940)", "pytest-mypy-plugins", "cloudpickle"]
+dev = ["cloudpickle", "coverage[toml] (>=5.0.2)", "furo", "hypothesis", "mypy (>=0.900,!=0.940)", "pre-commit", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "sphinx", "sphinx-notfound-page", "zope.interface"]
+docs = ["furo", "sphinx", "sphinx-notfound-page", "zope.interface"]
+tests = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "zope.interface"]
+tests_no_zope = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins"]
 
 [[package]]
 name = "black"
@@ -106,9 +106,9 @@ typing-extensions = {version = ">=3.6.4", markers = "python_version < \"3.8\""}
 zipp = ">=0.5"
 
 [package.extras]
-docs = ["sphinx", "jaraco.packaging (>=8.2)", "rst.linker (>=1.9)"]
+docs = ["jaraco.packaging (>=8.2)", "rst.linker (>=1.9)", "sphinx"]
 perf = ["ipython"]
-testing = ["pytest (>=6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest-cov", "pytest-enabler (>=1.0.1)", "packaging", "pep517", "pyfakefs", "flufl-flake8", "pytest-perf (>=0.9.2)", "pytest-black (>=0.3.7)", "pytest-mypy", "importlib-resources (>=1.3)"]
+testing = ["flufl.flake8", "importlib-resources (>=1.3)", "packaging", "pep517", "pyfakefs", "pytest (>=6)", "pytest-black (>=0.3.7)", "pytest-checkdocs (>=2.4)", "pytest-cov", "pytest-enabler (>=1.0.1)", "pytest-flake8", "pytest-mypy", "pytest-perf (>=0.9.2)"]
 
 [[package]]
 name = "iniconfig"
@@ -167,7 +167,7 @@ python-dateutil = ">=2.7.3"
 pytz = ">=2017.2"
 
 [package.extras]
-test = ["pytest (>=4.0.2)", "pytest-xdist", "hypothesis (>=3.58)"]
+test = ["hypothesis (>=3.58)", "pytest (>=4.0.2)", "pytest-xdist"]
 
 [[package]]
 name = "pathspec"
@@ -189,8 +189,8 @@ python-versions = ">=3.6"
 importlib-metadata = {version = ">=0.12", markers = "python_version < \"3.8\""}
 
 [package.extras]
-testing = ["pytest-benchmark", "pytest"]
-dev = ["tox", "pre-commit"]
+dev = ["pre-commit", "tox"]
+testing = ["pytest", "pytest-benchmark"]
 
 [[package]]
 name = "py"
@@ -317,8 +317,8 @@ optional = false
 python-versions = ">=3.6"
 
 [package.extras]
-docs = ["sphinx", "jaraco.packaging (>=8.2)", "rst.linker (>=1.9)"]
-testing = ["pytest (>=4.6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest-cov", "pytest-enabler (>=1.0.1)", "jaraco-itertools", "func-timeout", "pytest-black (>=0.3.7)", "pytest-mypy"]
+docs = ["jaraco.packaging (>=8.2)", "rst.linker (>=1.9)", "sphinx"]
+testing = ["func-timeout", "jaraco.itertools", "pytest (>=4.6)", "pytest-black (>=0.3.7)", "pytest-checkdocs (>=2.4)", "pytest-cov", "pytest-enabler (>=1.0.1)", "pytest-flake8", "pytest-mypy"]
 
 [metadata]
 lock-version = "1.1"


Test patch:
diff --git a/tests/test_necstdb.py b/tests/test_necstdb.py
index bb5e9f6..7e70c58 100644
--- a/tests/test_necstdb.py
+++ b/tests/test_necstdb.py
@@ -53,10 +53,11 @@ DATA4_HEADER = {
         {"key": "array", "format": "3d", "size": 24},
     ]
 }
-DATA5 = ([[b"abc", b"def", b"ghi"]] for _ in range(55))
+DATA5 = ([[b"abc", b"def", b"ghi"], b"jkl"] for _ in range(55))
 DATA5_HEADER = {
     "data": [
         {"key": "strArray", "format": "3s3s3s", "size": 9},
+        {"key": "strLenMismatch", "format": "5s", "size": 5},
     ]
 }
 
@@ -66,14 +67,17 @@ EXPECTED_DATA1_TUPLE = (3, -16, -32, -64)
 EXPECTED_DATA2_TUPLE = (3, 16, 32, 64)
 EXPECTED_DATA3_TUPLE = (0.32, 3, b"byte", b"c")
 EXPECTED_DATA4_TUPLE = (True, b"str", 3, TIME, TIME)
-EXPECTED_DATA5_TUPLE = (b"abc", b"def", b"ghi")
+EXPECTED_DATA5_TUPLE = (b"abc", b"def", b"ghi", b"jkl\x00\x00")
 
 # DTypes (int8, int16, ...) are not preserved.
 EXPECTED_DATA1_DICT = {"int8": 3, "int16": -16, "int32": -32, "int64": -64}
 EXPECTED_DATA2_DICT = {"uint8": 3, "uint16": 16, "uint32": 32, "uint64": 64}
 EXPECTED_DATA3_DICT = {"float32": 0.32, "float64": 3, "_byte": b"byte", "_char": b"c"}
 EXPECTED_DATA4_DICT = {"bool": True, "string": b"str", "array": (3, TIME, TIME)}
-EXPECTED_DATA5_DICT = {"strArray": (b"abc", b"def", b"ghi")}
+EXPECTED_DATA5_DICT = {
+    "strArray": (b"abc", b"def", b"ghi"),
+    "strLenMismatch": b"jkl\x00\x00",
+}
 
 # DTypes (int8, int16, ...) are not preserved.
 EXPECTED_DATA1_DF = pd.DataFrame(
@@ -88,7 +92,9 @@ EXPECTED_DATA3_DF = pd.DataFrame(
 EXPECTED_DATA4_DF = pd.DataFrame(
     [(True, b"str", [3, TIME, TIME])], columns=["bool", "string", "array"]
 )
-EXPECTED_DATA5_DF = pd.DataFrame([(["abc", "def", "ghi"],)], columns=["strArray"])
+EXPECTED_DATA5_DF = pd.DataFrame(
+    [(["abc", "def", "ghi"], "jkl\x00\x00")], columns=["strArray", "strLenMismatch"]
+)
 
 EXPECTED_DATA1_ARRAY = np.array(
     [(3, -16, -32, -64)],
@@ -107,8 +113,8 @@ EXPECTED_DATA4_ARRAY = np.array(
     dtype=[("bool", "?"), ("string", "S3"), ("array", "3f8")],
 )
 EXPECTED_DATA5_ARRAY = np.array(
-    [(["abc", "def", "ghi"],)],
-    dtype=[("strArray", "3S3")],
+    [(("abc", "def", "ghi"), "jkl\x00\x00")],
+    dtype=[("strArray", "(3,)S3"), ("strLenMismatch", "S5")],
 )
 
 EXPECTED_DATA1_BYTE = b"\x03\xf0\xff\xe0\xff\xff\xff\xc0\xff\xff\xff\xff\xff\xff\xff"
@@ -118,7 +124,7 @@ EXPECTED_DATA4_BYTE = (
     b"\x01str"
     b"\x00\x00\x00\x00\x00\x00\x08@\xea!\x1b\xc3\x1eJ\xd8A\xea!\x1b\xc3\x1eJ\xd8A"
 )
-EXPECTED_DATA5_BYTE = b"abcdefghi"
+EXPECTED_DATA5_BYTE = b"abcdefghijkl\x00\x00"
 
 
 @pytest.fixture(scope="module")
@@ -249,13 +255,13 @@ class TestReadDatabase:
         assert all(EXPECTED_DATA3_ARRAY == actual["data3"][3])
         assert all(EXPECTED_DATA4_ARRAY == actual["data4"][3])
         assert len(actual["data4"][3]["array"]) == 3
-        assert all(EXPECTED_DATA5_ARRAY == actual["data5"][3])
+        assert EXPECTED_DATA5_ARRAY == actual["data5"][3]
         assert len(actual["data5"][3]["strArray"]) == 3
 
     def test_read_as_bytes(self, db_path):
         db = necstdb.opendb(db_path)
         actual = {name: db.open_table(name).read(astype="raw") for name in table_name}
-        formats = ["<bhiq", "<BHIQ", "<fd4sc", "<?3s3d", "<3s3s3s"]
+        formats = ["<bhiq", "<BHIQ", "<fd4sc", "<?3s3d", "<3s3s3s5s"]
         unpacked = {
             k: tuple(struct.iter_unpack(fmt, v))
             for fmt, (k, v) in zip(formats, actual.items())
@@ -395,7 +401,7 @@ class TestMethods:
                 ("data2", 330, 22, 15, "<BHIQ"),
                 ("data3", 594, 33, 17, "<fd4sc"),
                 ("data4", 1364, 44, 28, "<?3s3d"),
-                ("data5", 495, 55, 9, "<3s3s3s"),
+                ("data5", 495, 55, 14, "<3s3s3s5s"),
             ],
             columns=[
                 "table name",
diff --git a/tests/test_necstdb_recover.py b/tests/test_necstdb_recover.py
index acaed8e..2cb853d 100644
--- a/tests/test_necstdb_recover.py
+++ b/tests/test_necstdb_recover.py
@@ -4,12 +4,19 @@ import pytest
 
 import necstdb
 
-EXAMPLE_DATA_PATH = pathlib.Path(".") / "tests" / "example_data"
+
+@pytest.fixture
+def db_path(tmp_path_factory) -> pathlib.Path:
+    """Path to temporary database directory."""
+    return tmp_path_factory.mktemp("test_db")
 
 
 class TestReadDatabase:
-    def test_read_db(self):
-        db = necstdb.opendb(EXAMPLE_DATA_PATH)
+
+    EXAMPLE_DATA_PATH = pathlib.Path(".") / "tests" / "example_data"
+
+    def test_read_db_with_invalid_format_specifier(self):
+        db = necstdb.opendb(self.EXAMPLE_DATA_PATH)
         _ = db.open_table("data4").read(astype="raw")
         with pytest.raises(ValueError):
             _ = db.open_table("data4").read(astype="tuple")
@@ -29,3 +36,26 @@ class TestReadDatabase:
         print(actual)
         actual = db.open_table("data4").recovered.read(astype="array")
         print(actual)
+
+    def test_ignore_trailing_pad_bytes(self, db_path):
+        header = {
+            "data": [
+                {"key": "data", "format": "5s", "size": 5},
+                {"key": "bool", "format": "?", "size": 1},
+            ]
+        }
+
+        db = necstdb.opendb(db_path, mode="w")
+        db.create_table("string_length_missepecified", header)
+        table = db.open_table("string_length_missepecified", mode="ab")
+
+        data = b"abc"
+        _ = table.append(data, True)
+        table.close()  # Close table to flush the data
+
+        table = db.open_table("string_length_missepecified").recovered
+        assert table.read(astype="raw")[:5] == data + b"\x00\x00"  # Won't be recovered
+        assert table.read(astype="tuple")[0][0] == data
+        assert table.read(astype="dict")[0]["data"] == data
+        assert table.read(astype="df")["data"].values[0] == data
+        assert table.read(astype="sa")["data"][0] == data