pxf: keep list, block and precision in dataset cells

CHANGELOG.md

@@ -13,6 +13,21 @@ format changes.
 
 ### Changed
 
+- **A row read as cells keeps its list and block values and its precision
+  suffixes** (#47), as protowire-go's `DatasetRow` keeps them. So a row from
+  `DatasetReader.next_or_none`, iteration or `Result.datasets`, passed to
+  `bind_row`, binds as `DatasetReader.scan` binds it. Before, a list or block
+  cell came back as `("unknown", None)` and `bind_row` refused it, and `1.5p80`
+  came back as `("float", "1.5")`, losing its precision. The public `Cell` type
+  changes:
+  - New kinds `("list", [cell, ...])` and `("block", [entry, ...])`. An entry
+    is a `(kind, key, value, quoted)` tuple of kind `"field"` (`k = v`),
+    `"map"` (`k: v`), `"block"` (`k { ... }`) or `"type"` (`@type = "url"`).
+    `quoted` records a key written as a string literal.
+  - An `"int"` or `"float"` cell's raw text keeps its precision suffix:
+    `("float", "1.5p80")`. **`float(cell[1])` now raises on such a cell**,
+    where before it silently dropped the precision.
+
 - **`pxf.bind_row` and `DatasetReader.scan` bind a row as protowire-go does**
   (#24), through protowire-cpp's `BindRow` rather than writing the row out as
   text and decoding it with `unmarshal`. On all 43 of the spec's
@@ -21,8 +36,8 @@ format changes.
   differently. What changes:
   - An absent cell takes the field's `(pxf.default)`. An absent
     `(pxf.required)` field is an error, as in `unmarshal_full`.
-  - A cell may hold a list or a block (`scan` only; see #47 for rows read as
-    cells), and a column may be a dotted path into a sub-message.
+  - A cell may hold a list or a block, and a column may be a dotted path
+    into a sub-message.
   - **A row binds into what the message already holds,** which is no longer
     cleared first. In a reused message, a field an earlier row set keeps its
     value under an empty or `null` cell, as in Go. Bind each row into a new or

src/_protowire/module.cc

@@ -85,20 +85,56 @@ const pbuf::Descriptor* FindDescriptor(const SchemaBundle& s,
 
 // CellToPyTuple converts a single AST cell value (or std::nullopt for an
 // absent cell) into the FFI shape consumed by pxf.py — `None` for absent,
-// `(kind, value)` otherwise. Used by PxfUnmarshalFull for @dataset rows.
+// `(kind, value)` otherwise. It keeps everything protowire-go's DatasetRow
+// cell keeps (#47).
 //
 // kind values mirror the AST variant tags:
 //   "null"      → nb::none()
 //   "string"    → str  (already-unescaped UTF-8)
-//   "int"       → str  (raw integer text — Python wrapper decides parse)
-//   "float"     → str  (raw float text)
+//   "int"       → str  (raw integer text, its precision suffix included:
+//   "float"     → str   "1.5p80". The parser keeps the suffix apart, in
+//                        prec; PXF numbers are decimal, so the 'p' is
+//                        unambiguous and PyCellToValue splits it off again.)
 //   "bool"      → bool
 //   "bytes"     → bytes
 //   "ident"     → str
 //   "timestamp" → str  (raw RFC3339)
 //   "duration"  → str  (raw duration)
-nb::object CellToPyTuple(const std::optional<protowire::pxf::ValuePtr>& cell) {
-  if (!cell.has_value()) return nb::none();
+//   "list"      → list of cells (an element is never absent)
+//   "block"     → list of entries, each (kind, key, value, quoted):
+//                   ("field", name, cell, quoted)   name = value
+//                   ("map", key, cell, quoted)      key: value
+//                   ("block", name, [entry, ...], quoted)  name { ... }
+//                   ("type", url, None, False)      @type = "url"
+//                 quoted records a key written as a string literal.
+nb::object ValueToPy(const protowire::pxf::ValuePtr& value);
+
+nb::object EntryToPy(const protowire::pxf::EntryPtr& entry) {
+  using namespace protowire::pxf;
+  return std::visit(
+      [](const auto& p) -> nb::object {
+        using T = std::decay_t<decltype(*p)>;
+        if constexpr (std::is_same_v<T, Assignment>) {
+          return nb::make_tuple(std::string("field"), p->key, ValueToPy(p->value), p->key_quoted);
+        } else if constexpr (std::is_same_v<T, MapEntry>) {
+          return nb::make_tuple(std::string("map"), p->key, ValueToPy(p->value), p->key_quoted);
+        } else if constexpr (std::is_same_v<T, Block>) {
+          nb::list entries;
+          for (const auto& e : p->entries) entries.append(EntryToPy(e));
+          return nb::make_tuple(std::string("block"), p->name, entries, p->name_quoted);
+        } else {
+          static_assert(std::is_same_v<T, TypeEntry>);
+          return nb::make_tuple(std::string("type"), p->url, nb::none(), false);
+        }
+      },
+      entry);
+}
+
+std::string WithPrec(const std::string& raw, const std::string& prec) {
+  return prec.empty() ? raw : raw + "p" + prec;
+}
+
+nb::object ValueToPy(const protowire::pxf::ValuePtr& value) {
   using namespace protowire::pxf;
   return std::visit(
       [](const auto& p) -> nb::object {
@@ -108,9 +144,9 @@ nb::object CellToPyTuple(const std::optional<protowire::pxf::ValuePtr>& cell) {
         } else if constexpr (std::is_same_v<T, StringVal>) {
           return nb::make_tuple(std::string("string"), p->value);
         } else if constexpr (std::is_same_v<T, IntVal>) {
-          return nb::make_tuple(std::string("int"), p->raw);
+          return nb::make_tuple(std::string("int"), WithPrec(p->raw, p->prec));
         } else if constexpr (std::is_same_v<T, FloatVal>) {
-          return nb::make_tuple(std::string("float"), p->raw);
+          return nb::make_tuple(std::string("float"), WithPrec(p->raw, p->prec));
         } else if constexpr (std::is_same_v<T, BoolVal>) {
           return nb::make_tuple(std::string("bool"), p->value);
         } else if constexpr (std::is_same_v<T, BytesVal>) {
@@ -123,13 +159,26 @@ nb::object CellToPyTuple(const std::optional<protowire::pxf::ValuePtr>& cell) {
           return nb::make_tuple(std::string("timestamp"), p->raw);
         } else if constexpr (std::is_same_v<T, DurationVal>) {
           return nb::make_tuple(std::string("duration"), p->raw);
+        } else if constexpr (std::is_same_v<T, ListVal>) {
+          nb::list elements;
+          for (const auto& e : p->elements) elements.append(ValueToPy(e));
+          return nb::make_tuple(std::string("list"), elements);
+        } else if constexpr (std::is_same_v<T, BlockVal>) {
+          nb::list entries;
+          for (const auto& e : p->entries) entries.append(EntryToPy(e));
+          return nb::make_tuple(std::string("block"), entries);
         } else {
-          // List / Block are rejected at @dataset cell-parse time, so this
-          // branch is unreachable for cells. Surface as a clean error.
+          // BadVal: a parse error's placeholder. A reader that returned the
+          // row has already reported the error, so this is not reached.
           return nb::make_tuple(std::string("unknown"), nb::none());
         }
       },
-      *cell);
+      value);
+}
+
+nb::object CellToPyTuple(const std::optional<protowire::pxf::ValuePtr>& cell) {
+  if (!cell.has_value()) return nb::none();
+  return ValueToPy(*cell);
 }
 
 // The decode limits of HARDENING.md § Mandatory limits, as Python passes them:
@@ -159,11 +208,61 @@ void ApplyLimits(const PxfLimits& l, protowire::pxf::UnmarshalOptions& opts) {
 
 // PyCellToValue is CellToPyTuple's inverse, for a cell pxf.py has already
 // normalized: None for absent, else (kind, value) with value a str, a bool
-// ("bool") or bytes ("bytes"). A value cell that crossed to Python as
-// ("unknown", None), a list or a block, cannot come back.
-std::optional<protowire::pxf::ValuePtr> PyCellToValue(nb::handle cell) {
+// ("bool"), bytes ("bytes"), a list of cells ("list") or a list of
+// (kind, key, value, quoted) entries ("block"). An "int" or "float" raw
+// text's precision suffix goes back to prec.
+protowire::pxf::ValuePtr PyToValue(nb::handle cell);
+
+protowire::pxf::EntryPtr PyToEntry(nb::handle entry) {
   using namespace protowire::pxf;
-  if (cell.is_none()) return std::nullopt;
+  auto t = nb::cast<nb::tuple>(entry);
+  if (t.size() != 4) {
+    throw nb::value_error("pxf.bind_row: a block entry is a (kind, key, value, quoted) tuple");
+  }
+  const auto kind = nb::cast<std::string>(t[0]);
+  auto key = nb::cast<std::string>(t[1]);
+  const bool quoted = nb::cast<bool>(t[3]);
+  if (kind == "field") {
+    auto p = std::make_unique<Assignment>();
+    p->key = std::move(key);
+    p->key_quoted = quoted;
+    p->value = PyToValue(t[2]);
+    return EntryPtr(std::move(p));
+  }
+  if (kind == "map") {
+    auto p = std::make_unique<MapEntry>();
+    p->key = std::move(key);
+    p->key_quoted = quoted;
+    p->value = PyToValue(t[2]);
+    return EntryPtr(std::move(p));
+  }
+  if (kind == "block") {
+    auto p = std::make_unique<Block>();
+    p->name = std::move(key);
+    p->name_quoted = quoted;
+    for (nb::handle e : nb::cast<nb::list>(t[2])) p->entries.push_back(PyToEntry(e));
+    return EntryPtr(std::move(p));
+  }
+  if (kind == "type") {
+    auto p = std::make_unique<TypeEntry>();
+    p->url = std::move(key);
+    return EntryPtr(std::move(p));
+  }
+  throw nb::value_error(("pxf.bind_row: unknown block entry kind '" + kind + "'").c_str());
+}
+
+// SplitPrec splits "1.5p80" into raw "1.5" and prec "80". PXF numbers are
+// decimal, so a 'p' can only start the suffix.
+template <class T>
+void SplitPrec(const std::string& text, T& v) {
+  const auto at = text.find('p');
+  v.raw = text.substr(0, at);
+  if (at != std::string::npos) v.prec = text.substr(at + 1);
+}
+
+protowire::pxf::ValuePtr PyToValue(nb::handle cell) {
+  using namespace protowire::pxf;
+  if (cell.is_none()) throw nb::value_error("pxf.bind_row: a list element cannot be absent");
   auto t = nb::cast<nb::tuple>(cell);
   if (t.size() != 2) throw nb::value_error("pxf.bind_row: a cell is None or a (kind, value) pair");
   const auto kind = nb::cast<std::string>(t[0]);
@@ -176,12 +275,12 @@ std::optional<protowire::pxf::ValuePtr> PyCellToValue(nb::handle cell) {
   }
   if (kind == "int") {
     auto p = std::make_unique<IntVal>();
-    p->raw = nb::cast<std::string>(v);
+    SplitPrec(nb::cast<std::string>(v), *p);
     return ValuePtr(std::move(p));
   }
   if (kind == "float") {
     auto p = std::make_unique<FloatVal>();
-    p->raw = nb::cast<std::string>(v);
+    SplitPrec(nb::cast<std::string>(v), *p);
     return ValuePtr(std::move(p));
   }
   if (kind == "bool") {
@@ -211,9 +310,24 @@ std::optional<protowire::pxf::ValuePtr> PyCellToValue(nb::handle cell) {
     p->raw = nb::cast<std::string>(v);
     return ValuePtr(std::move(p));
   }
+  if (kind == "list") {
+    auto p = std::make_unique<ListVal>();
+    for (nb::handle e : nb::cast<nb::list>(v)) p->elements.push_back(PyToValue(e));
+    return ValuePtr(std::move(p));
+  }
+  if (kind == "block") {
+    auto p = std::make_unique<BlockVal>();
+    for (nb::handle e : nb::cast<nb::list>(v)) p->entries.push_back(PyToEntry(e));
+    return ValuePtr(std::move(p));
+  }
   throw nb::value_error(("pxf.bind_row: unknown cell kind '" + kind + "'").c_str());
 }
 
+std::optional<protowire::pxf::ValuePtr> PyCellToValue(nb::handle cell) {
+  if (cell.is_none()) return std::nullopt;
+  return PyToValue(cell);
+}
+
 // NewMessageFrom makes a message of desc holding msg_bytes. A row binds into
 // what the message already holds, as protowire-go's BindRow and Scan do: they
 // do not reset it first.

src/protowire/pxf.py

@@ -28,17 +28,38 @@
 #   ----        ----------        -----
 #   "null"      None              present-but-null (draft §3.9)
 #   "string"    str               escape-decoded UTF-8
-#   "int"       str               raw text — Python wrapper leaves parse to caller
-#   "float"     str               raw text
+#   "int"       str               raw text — Python wrapper leaves parse to caller.
+#                                 A precision suffix is kept: "1.5p80".
+#   "float"     str               raw text, as for "int"
 #   "bool"      bool              true / false
 #   "bytes"     bytes             base64-decoded
 #   "ident"     str               unquoted identifier (typically an enum tag name)
 #   "timestamp" str               raw RFC3339
 #   "duration"  str               raw duration text
+#   "list"      list[Cell]        `[a, b]`; an element is never None
+#   "block"     list[Entry]       `{ ... }`, its entries in order
+#
+# An Entry is a (kind, key, value, quoted) tuple:
+#
+#   kind        key           value         written as
+#   ----        ---           -----         ----------
+#   "field"     field name    Cell          name = value
+#   "map"       map key       Cell          key: value
+#   "block"     field name    list[Entry]   name { ... }
+#   "type"      type URL      None          @type = "url"
+#
+# quoted is True when the key was written as a string literal (`"k": v`),
+# which tells the string map key "1" from the integer key 1.
+#
+# This keeps what protowire-go's DatasetRow keeps (#47), so a row read as
+# cells binds with bind_row as DatasetReader.scan binds it.
 CellKind = Literal[
-    "null", "string", "int", "float", "bool", "bytes", "ident", "timestamp", "duration"
+    "null", "string", "int", "float", "bool", "bytes", "ident", "timestamp", "duration",
+    "list", "block",
 ]
+EntryKind = Literal["field", "map", "block", "type"]
 Cell = Union[None, tuple[CellKind, object]]
+Entry = tuple[EntryKind, str, object, bool]
 
 
 @dataclass(frozen=True)
@@ -617,9 +638,16 @@ def bind_row(
 
 def _native_cell(cell: Cell | None):
     """A cell in the shape the native bind_row takes: None, or (kind, value)
-    with value a str, or a bool for "bool" and bytes for "bytes"."""
+    with value a str, a bool for "bool", bytes for "bytes", a list of cells
+    for "list" and a list of entries for "block"."""
     if cell is None:
         return None
+    return _native_value(cell)
+
+
+def _native_value(cell):
+    if cell is None:
+        raise ValueError("bind_row: a list element cannot be absent (None)")
     kind, value = cell
     if kind == "null":
         return ("null", None)
@@ -629,4 +657,19 @@ def _native_cell(cell: Cell | None):
         return (kind, bool(value))
     if kind == "bytes":
         return (kind, bytes(value))
+    if kind == "list":
+        return (kind, [_native_value(e) for e in value])
+    if kind == "block":
+        return (kind, [_native_entry(e) for e in value])
     raise ValueError(f"bind_row: unknown cell kind {kind!r}")
+
+
+def _native_entry(entry):
+    kind, key, value, quoted = entry
+    if kind in ("field", "map"):
+        return (kind, str(key), _native_value(value), bool(quoted))
+    if kind == "block":
+        return (kind, str(key), [_native_entry(e) for e in value], bool(quoted))
+    if kind == "type":
+        return (kind, str(key), None, False)
+    raise ValueError(f"bind_row: unknown block entry kind {kind!r}")