eugenegujing commented on code in PR #6278:
URL: https://github.com/apache/texera/pull/6278#discussion_r3556179607
##########
amber/src/main/python/core/models/tuple.py:
##########
@@ -324,30 +327,38 @@ def cast_to_schema(self, schema: Schema) -> None:
self[field_name] = None
elif field_value is not None:
field_type = schema.get_attr_type(field_name)
- if (
- field_type in INTEGRAL_TYPE_RANGES
- and isinstance(field_value, float)
- and field_value.is_integer()
+ if field_type in INTEGRAL_TYPE_RANGES and (
+ isinstance(field_value, numpy.integer)
+ or (isinstance(field_value, float) and
field_value.is_integer())
):
- # pandas 2.2.3 promotes an int column holding nulls to
- # float64 (119 -> 119.0), so convert integral floats
- # destined for INT/LONG back to int — but only within
- # the safe range above; out-of-range floats are left
- # unchanged so validation still fails. Compare on the
- # int result to avoid float rounding at the endpoints.
+ # Integer-valued inputs reach INT/LONG as non-int
types:
Review Comment:
fixed
##########
amber/src/test/python/core/models/test_tuple.py:
##########
@@ -305,6 +305,169 @@ def test_finalize_maps_nan_in_binary_field_to_none(self):
tuple_.finalize(Schema(raw_schema={"payload": "BINARY"}))
assert tuple_["payload"] is None
+ # Pandas-based operators also produce numpy scalar types very naturally:
+ # reductions such as df["x"].sum()/.max()/.count() return numpy.int64, and
+ # df["x"].any() or any numpy comparison returns numpy.bool_. These are NOT
+ # subclasses of Python int/bool, so finalize() must coerce them for
INT/LONG
+ # and BOOL fields the same way it already coerces integral floats — while
+ # still rejecting numpy integers outside the target range and never
crossing
+ # the bool<->int boundary.
+
+ @pytest.mark.parametrize(
+ "raw_value, attr_type, expected",
+ [
+ (np.int64(5), "INTEGER", 5),
+ (np.int32(5), "INTEGER", 5),
+ (np.int64(-7), "INTEGER", -7),
+ # int32 boundaries fit an INT field
+ (np.int64(2**31 - 1), "INTEGER", 2**31 - 1),
+ (np.int64(-(2**31)), "INTEGER", -(2**31)),
+ # an np.int64 that overflows int32 still fits a LONG field
+ (np.int64(3000000000), "LONG", 3000000000),
+ ],
+ )
+ def test_finalize_coerces_numpy_integer_to_int(
+ self, raw_value, attr_type, expected
+ ):
+ tuple_ = Tuple({"count": raw_value})
+ tuple_.finalize(Schema(raw_schema={"count": attr_type}))
+ assert tuple_["count"] == expected
+ assert type(tuple_["count"]) is int
+
+ @pytest.mark.parametrize(
+ "raw_value, expected",
+ [(np.bool_(True), True), (np.bool_(False), False)],
+ )
+ def test_finalize_coerces_numpy_bool_to_bool(self, raw_value, expected):
+ tuple_ = Tuple({"flag": raw_value})
+ tuple_.finalize(Schema(raw_schema={"flag": "BOOLEAN"}))
+ assert tuple_["flag"] == expected
+ assert type(tuple_["flag"]) is bool
+
+ def test_cast_to_schema_coerces_numpy_integer(self):
+ # The coercion must live in cast_to_schema(), mirroring the
+ # integral-float coercion.
+ tuple_ = Tuple({"count": np.int64(5)})
+ tuple_.cast_to_schema(Schema(raw_schema={"count": "INTEGER"}))
+ assert tuple_["count"] == 5
+ assert type(tuple_["count"]) is int
+
+ def test_cast_to_schema_coerces_numpy_bool(self):
+ tuple_ = Tuple({"flag": np.bool_(True)})
+ tuple_.cast_to_schema(Schema(raw_schema={"flag": "BOOLEAN"}))
+ assert tuple_["flag"] is True
+ assert type(tuple_["flag"]) is bool
+
+ @pytest.mark.parametrize(
+ "raw_value",
+ [np.int64(2**31), np.int64(-(2**31) - 1)],
+ )
+ def test_finalize_rejects_out_of_range_numpy_integer(self, raw_value):
+ # An np.int64 outside int32 range must be left unchanged so validation
+ # still fails; it must never silently overflow int32.
+ tuple_ = Tuple({"count": raw_value})
+ with pytest.raises(TypeError, match="Unmatched type"):
+ tuple_.finalize(Schema(raw_schema={"count": "INTEGER"}))
+
+ def test_finalize_rejects_numpy_bool_in_int_field(self):
+ # Coercion must never cross the bool<->int boundary: np.bool_ is not
+ # np.integer, so it must not be coerced into an INT field.
+ tuple_ = Tuple({"count": np.bool_(True)})
+ with pytest.raises(TypeError, match="Unmatched type"):
+ tuple_.finalize(Schema(raw_schema={"count": "INTEGER"}))
+
+ def test_finalize_keeps_plain_bool_in_int_field_unchanged(self):
+ # Pin the pre-existing behavior: a plain Python bool passes INT
+ # validation (bool subclasses int) and is left as a bool.
+ tuple_ = Tuple({"flag": True})
+ tuple_.finalize(Schema(raw_schema={"flag": "INTEGER"}))
+ assert tuple_["flag"] is True
+ assert type(tuple_["flag"]) is bool
+
+ def test_finalize_keeps_plain_bool_unchanged(self):
+ tuple_ = Tuple({"flag": True})
+ tuple_.finalize(Schema(raw_schema={"flag": "BOOLEAN"}))
+ assert tuple_["flag"] is True
+ assert type(tuple_["flag"]) is bool
+
+ def test_finalize_coerces_numpy_scalars_from_pandas_reduction(self):
+ # Mirrors idiomatic pandas UDF output: df["x"].sum() returns
+ # numpy.int64 and (df["x"] > n).any() returns numpy.bool_. Both must be
+ # accepted and stored as Python builtins.
+ df = pandas.DataFrame({"age": [20, 65, 70]})
+ tuple_ = Tuple(
+ {
+ "total_age": df["age"].sum(),
+ "has_senior": (df["age"] > 60).any(),
+ }
+ )
+ assert isinstance(tuple_["total_age"], np.integer)
+ assert isinstance(tuple_["has_senior"], np.bool_)
+ tuple_.finalize(
+ Schema(raw_schema={"total_age": "INTEGER", "has_senior":
"BOOLEAN"})
+ )
+ assert type(tuple_["total_age"]) is int
+ assert tuple_["total_age"] == 155
+ assert type(tuple_["has_senior"]) is bool
+ assert tuple_["has_senior"] is True
+
+ @pytest.mark.parametrize("raw_value", [np.int64(1), np.int64(0)])
+ def test_finalize_rejects_numpy_integer_in_bool_field(self, raw_value):
+ # Symmetric guard to the bool<->int boundary: a numpy integer must
+ # never be coerced into a BOOLEAN field. The BOOL branch is gated on
+ # isinstance(v, numpy.bool_), and numpy.integer is not numpy.bool_.
+ tuple_ = Tuple({"flag": raw_value})
+ with pytest.raises(TypeError, match="Unmatched type"):
+ tuple_.finalize(Schema(raw_schema={"flag": "BOOLEAN"}))
+
+ @pytest.mark.parametrize(
+ "raw_value, expected",
+ [
+ # both ends of the float64 exact-integer window map to a unique
+ # float64, so they must be coerced to Python int
+ (np.int64(2**53 - 1), 2**53 - 1),
+ (np.int64(-(2**53) + 1), -(2**53) + 1),
+ ],
+ )
+ def test_finalize_coerces_numpy_integer_at_long_window_edge(
+ self, raw_value, expected
+ ):
+ tuple_ = Tuple({"count": raw_value})
+ tuple_.finalize(Schema(raw_schema={"count": "LONG"}))
+ assert tuple_["count"] == expected
+ assert type(tuple_["count"]) is int
+
+ @pytest.mark.parametrize("raw_value", [np.int64(2**53),
np.int64(-(2**53))])
+ def test_finalize_rejects_numpy_integer_beyond_long_window(self,
raw_value):
+ # Just outside the float64 exact-integer window: even though these fit
+ # int64, LONG is bounded by the 2**53 window, so they must be left
+ # unchanged and fail validation rather than silently coerced.
+ tuple_ = Tuple({"count": raw_value})
+ with pytest.raises(TypeError, match="Unmatched type"):
+ tuple_.finalize(Schema(raw_schema={"count": "LONG"}))
+
+ def test_finalize_coerces_unsigned_numpy_integer_to_int(self):
+ # Unsigned numpy integers are also numpy.integer, and int() is exact
+ # for them, so an in-range uint must be coerced to a Python int.
+ tuple_ = Tuple({"count": np.uint32(5)})
+ tuple_.finalize(Schema(raw_schema={"count": "INTEGER"}))
+ assert tuple_["count"] == 5
+ assert type(tuple_["count"]) is int
+
+ def test_finalize_rejects_unsigned_numpy_integer_beyond_long_window(self):
+ # A uint64 above the float64 exact-integer window must not be coerced;
+ # the range check catches large unsigned values the same way.
+ tuple_ = Tuple({"count": np.uint64(2**60)})
+ with pytest.raises(TypeError, match="Unmatched type"):
+ tuple_.finalize(Schema(raw_schema={"count": "LONG"}))
+
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
