Copilot commented on code in PR #3841:
URL: https://github.com/apache/iceberg-python/pull/3841#discussion_r3847293272
##########
tests/test_transforms.py:
##########
@@ -1710,3 +1710,21 @@ def
test_calling_pyarrow_transform_without_pyiceberg_core_installed_correctly_ra
with pytest.raises(NotInstalledError):
transform.pyarrow_transform(StringType())
+
+
+def test_pyarrow_transforms_dictionary_encoded() -> None:
+ dict_arr = pa.DictionaryArray.from_arrays(pa.array([0, 1, 0, None]),
pa.array(["foo", "bar"]))
+ raw_arr = pa.array(["foo", "bar", "foo", None])
+ bucket_transform = BucketTransform(num_buckets=10)
+ expected_bucket = bucket_transform.pyarrow_transform(StringType())(raw_arr)
+ assert bucket_transform.pyarrow_transform(StringType())(dict_arr) ==
expected_bucket
Review Comment:
_pyiceberg_transform_wrapper now normalizes dictionary-encoded arrays for
all PyArrow transforms, including Year/Month/Day which also exercise the
`expected_type` casting path. The added regression test covers Bucket/Truncate,
but does not cover a time transform (or any transform using `expected_type`),
so the new behavior is still untested for that wrapper branch.
##########
tests/test_transforms.py:
##########
@@ -1710,3 +1710,21 @@ def
test_calling_pyarrow_transform_without_pyiceberg_core_installed_correctly_ra
with pytest.raises(NotInstalledError):
transform.pyarrow_transform(StringType())
+
+
+def test_pyarrow_transforms_dictionary_encoded() -> None:
+ dict_arr = pa.DictionaryArray.from_arrays(pa.array([0, 1, 0, None]),
pa.array(["foo", "bar"]))
+ raw_arr = pa.array(["foo", "bar", "foo", None])
+ bucket_transform = BucketTransform(num_buckets=10)
+ expected_bucket = bucket_transform.pyarrow_transform(StringType())(raw_arr)
+ assert bucket_transform.pyarrow_transform(StringType())(dict_arr) ==
expected_bucket
+
+ chunked_dict = pa.chunked_array([dict_arr, dict_arr])
+ expected_chunked = pa.chunked_array([expected_bucket, expected_bucket])
+ assert bucket_transform.pyarrow_transform(StringType())(chunked_dict) ==
expected_chunked
+
+ truncate_transform = TruncateTransform(width=3)
+ dict_truncate_arr = pa.DictionaryArray.from_arrays(pa.array([0, 1, 0]),
pa.array(["developer", "iceberg"]))
Review Comment:
The new dictionary-encoded test uses default index types, which currently
yields Dictionary(Int64, Utf8) in practice. The reported failure was
Dictionary(Int32, Utf8); specifying the indices array type explicitly (for both
dict arrays in this test) ensures the regression test matches the real-world
case and covers both index widths deterministically.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
