pr feedback: none default values per type

alan-gauthier-jt · alan-gauthier-jt · commit d21d32cfdd1b · 2026-05-07T11:12:26.000+02:00
Signed-off-by: Alan Gauthier &lt;alan.gauthier@jobteaser.com&gt;
diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py
@@ -825,43 +825,38 @@ def convert_set_to_list(value: Any) -> Any:
     ]
 
 
-# Sentinel value used by _to_proto_safe_list to indicate that None elements
-# should simply be filtered (dropped) rather than replaced with a default.
-_DROP_NONE = object()
-
 # Per-type default values substituted for None elements inside list columns.
-# Only STRING_LIST uses ""; numeric/bytes types drop None entirely because
-# there is no meaningful in-band sentinel (protobuf rejects wrong scalar types).
-_LIST_TYPE_NONE_REPLACEMENT: Dict[ValueType, Any] = {
+# Protobuf repeated fields do not accept None, so we replace with a
+# type-appropriate zero/empty value.
+_LIST_NONE_DEFAULTS: Dict[ValueType, Any] = {
     ValueType.STRING_LIST: "",
+    ValueType.BYTES_LIST: b"",
+    ValueType.INT32_LIST: 0,
+    ValueType.INT64_LIST: 0,
+    ValueType.FLOAT_LIST: 0.0,
+    ValueType.DOUBLE_LIST: 0.0,
+    ValueType.BOOL_LIST: False,
+    ValueType.UNIX_TIMESTAMP_LIST: NULL_TIMESTAMP_INT_VALUE,
+    ValueType.UUID_LIST: "",
+    ValueType.TIME_UUID_LIST: "",
+    ValueType.DECIMAL_LIST: "",
 }
 
 
-def _to_proto_safe_list(
-    value: Any, feast_value_type: ValueType = ValueType.STRING_LIST
-) -> Any:
-    """Convert an array/list column value to a proto-safe Python list.
-
-    Arrow/Athena returns Array columns as numpy.ndarray (object dtype).
-    Protobuf repeated fields reject ndarrays and (for non-string types) None
-    elements, so we:
-    1. Call .tolist() to convert any numpy.ndarray to a plain Python list.
-    2. For STRING_LIST: replace None elements with "" (empty string).
-       For all other list types: drop None elements, since there is no valid
-       in-band default for numeric/bytes protobuf fields.
+def _sanitize_list_value(value: Any, feast_value_type: ValueType) -> Any:
+    """Convert ndarray to list and replace None elements with a type-appropriate default.
 
-    Args:
-        value: The raw column value (ndarray, list, or scalar).
-        feast_value_type: The Feast ValueType of the list column. Controls how
-            None elements are handled. Defaults to STRING_LIST.
+    Arrow/Athena may deserialize array columns as numpy.ndarray with object dtype
+    instead of plain Python lists.  Protobuf repeated fields do not accept ndarrays
+    or None elements, so we normalise here before building proto messages.
     """
     if isinstance(value, np.ndarray):
         value = value.tolist()
-    if isinstance(value, list):
-        none_replacement = _LIST_TYPE_NONE_REPLACEMENT.get(feast_value_type, _DROP_NONE)
-        if none_replacement is _DROP_NONE:
-            return [x for x in value if x is not None]
-        return [x if x is not None else none_replacement for x in value]
+        if isinstance(value, list) and len(value) == 0:
+            return None
+    none_default = _LIST_NONE_DEFAULTS.get(feast_value_type)
+    if none_default is not None and isinstance(value, list):
+        value = [none_default if v is None else v for v in value]
     return value
 
 
@@ -887,6 +882,13 @@ def _convert_list_values_to_proto(
         feast_value_type
     ]
 
+    values = [
+        _sanitize_list_value(v, feast_value_type) if v is not None else v
+        for v in values
+    ]
+    if sample is not None:
+        sample = _sanitize_list_value(sample, feast_value_type)
+
     # Bytes to array type conversion
     if isinstance(sample, (bytes, bytearray)):
         if feast_value_type == ValueType.BYTES_LIST:
@@ -943,14 +945,8 @@ def _convert_list_values_to_proto(
         ]
 
     # Generic list conversion
-    # Arrow/Athena deserializes Array columns as numpy.ndarray (object dtype).
-    # _to_proto_safe_list converts to a plain Python list and sanitizes None
-    # elements in a type-appropriate way (replaced with "" for STRING_LIST,
-    # dropped for numeric/bytes types).
     return [
-        ProtoValue(
-            **{field_name: proto_type(val=_to_proto_safe_list(value, feast_value_type))}  # type: ignore[arg-type]
-        )
+        ProtoValue(**{field_name: proto_type(val=value)})  # type: ignore[arg-type]
         if value is not None
         else ProtoValue()
         for value in values
diff --git a/sdk/python/tests/unit/test_type_map.py b/sdk/python/tests/unit/test_type_map.py
@@ -1967,77 +1967,87 @@ class TestArrowArrayStringListMaterialization:
     2. TypeError: "bad argument type for built-in operation"
        — when proto_type(val=<ndarray>) was called; protobuf rejects ndarrays.
 
-    Both are fixed by _to_proto_safe_list, which converts ndarrays to plain Python
-    lists and sanitizes None elements in a type-appropriate way:
-    - STRING_LIST: None → "" (empty string)
-    - All other list types: None elements are dropped (filtered out)
+    Both are fixed by _sanitize_list_value, which converts ndarrays to plain Python
+    lists and replaces None elements with a type-appropriate zero/empty default
+    (see _LIST_NONE_DEFAULTS).
     """
 
-    def test_to_proto_safe_list_ndarray(self):
+    def test_sanitize_list_value_ndarray(self):
         """ndarray is converted to a plain Python list."""
-        from feast.type_map import _to_proto_safe_list
+        from feast.type_map import _sanitize_list_value
 
         arr = np.array(["foo", "bar"], dtype=object)
-        result = _to_proto_safe_list(arr)
+        result = _sanitize_list_value(arr, ValueType.STRING_LIST)
         assert result == ["foo", "bar"]
         assert isinstance(result, list)
 
-    def test_to_proto_safe_list_empty_ndarray(self):
-        """Empty ndarray is converted to an empty list."""
-        from feast.type_map import _to_proto_safe_list
+    def test_sanitize_list_value_empty_ndarray(self):
+        """Empty ndarray is converted to None (treated as a missing row)."""
+        from feast.type_map import _sanitize_list_value
 
         arr = np.array([], dtype=object)
-        result = _to_proto_safe_list(arr)
-        assert result == []
-        assert isinstance(result, list)
+        result = _sanitize_list_value(arr, ValueType.STRING_LIST)
+        assert result is None
 
-    def test_to_proto_safe_list_ndarray_with_none(self):
+    def test_sanitize_list_value_ndarray_with_none(self):
         """None elements inside a STRING_LIST ndarray are replaced with empty string."""
-        from feast.type_map import _to_proto_safe_list
+        from feast.type_map import _sanitize_list_value
 
         arr = np.array(["foo", None, "baz"], dtype=object)
-        result = _to_proto_safe_list(arr, ValueType.STRING_LIST)
+        result = _sanitize_list_value(arr, ValueType.STRING_LIST)
         assert result == ["foo", "", "baz"]
 
-    def test_to_proto_safe_list_plain_list(self):
-        """Plain Python lists pass through unchanged (no None replacement needed)."""
-        from feast.type_map import _to_proto_safe_list
+    def test_sanitize_list_value_plain_list(self):
+        """Plain Python lists without None pass through unchanged."""
+        from feast.type_map import _sanitize_list_value
 
         lst = ["foo", "bar"]
-        result = _to_proto_safe_list(lst)
+        result = _sanitize_list_value(lst, ValueType.STRING_LIST)
         assert result == ["foo", "bar"]
 
-    def test_to_proto_safe_list_plain_list_with_none(self):
+    def test_sanitize_list_value_plain_list_with_none(self):
         """None elements in a STRING_LIST plain list are replaced with empty string."""
-        from feast.type_map import _to_proto_safe_list
+        from feast.type_map import _sanitize_list_value
 
         lst = ["foo", None]
-        result = _to_proto_safe_list(lst, ValueType.STRING_LIST)
+        result = _sanitize_list_value(lst, ValueType.STRING_LIST)
         assert result == ["foo", ""]
 
-    def test_to_proto_safe_list_numeric_list_none_dropped(self):
-        """None elements in non-string lists are dropped, not replaced with a sentinel."""
-        from feast.type_map import _to_proto_safe_list
+    def test_sanitize_list_value_numeric_none_replaced(self):
+        """None elements in numeric lists are replaced with a type-appropriate default."""
+        from feast.type_map import _sanitize_list_value
 
-        for vt in (
-            ValueType.FLOAT_LIST,
-            ValueType.DOUBLE_LIST,
-            ValueType.INT32_LIST,
-            ValueType.INT64_LIST,
-            ValueType.BYTES_LIST,
-        ):
-            result = _to_proto_safe_list([1.0, None, 2.0], vt)
-            assert result == [1.0, 2.0], (
-                f"Expected None dropped for {vt}, got {result!r}"
-            )
+        assert _sanitize_list_value([1, None, 2], ValueType.INT32_LIST) == [1, 0, 2]
+        assert _sanitize_list_value([1, None, 2], ValueType.INT64_LIST) == [1, 0, 2]
+        assert _sanitize_list_value([1.0, None, 2.0], ValueType.FLOAT_LIST) == [
+            1.0,
+            0.0,
+            2.0,
+        ]
+        assert _sanitize_list_value([1.0, None, 2.0], ValueType.DOUBLE_LIST) == [
+            1.0,
+            0.0,
+            2.0,
+        ]
+        assert _sanitize_list_value([True, None, False], ValueType.BOOL_LIST) == [
+            True,
+            False,
+            False,
+        ]
+
+    def test_sanitize_list_value_bytes_none_replaced(self):
+        """None elements in BYTES_LIST are replaced with b''."""
+        from feast.type_map import _sanitize_list_value
+
+        result = _sanitize_list_value([b"x", None], ValueType.BYTES_LIST)
+        assert result == [b"x", b""]
 
-    def test_to_proto_safe_list_scalar_passthrough(self):
+    def test_sanitize_list_value_scalar_passthrough(self):
         """Non-list, non-ndarray values are returned unchanged."""
-        from feast.type_map import _to_proto_safe_list
+        from feast.type_map import _sanitize_list_value
 
-        assert _to_proto_safe_list("hello") == "hello"
-        assert _to_proto_safe_list(None) is None
-        assert _to_proto_safe_list(42) == 42
+        assert _sanitize_list_value("hello", ValueType.STRING_LIST) == "hello"
+        assert _sanitize_list_value(42, ValueType.INT32_LIST) == 42
 
     def test_string_list_from_ndarray(self):
         """STRING_LIST column with ndarray values materializes without TypeError."""
