pitrou commented on a change in pull request #10199:
URL: https://github.com/apache/arrow/pull/10199#discussion_r630955362
##########
File path: python/pyarrow/tests/test_array.py
##########
@@ -2666,6 +2666,38 @@ def test_array_masked():
assert arr.type == pa.int64()
+def test_binary_array_masked():
+ # ARROW-12431
+ masked_basic = pa.array([b'\x05'], type=pa.binary(1),
+ mask=np.array([False]))
+ assert [b'\x05'] == masked_basic.to_pylist()
+
+ # Fixed Length Binary
+ masked = pa.array(np.array([b'\x05']), type=pa.binary(1),
+ mask=np.array([False]))
+ assert [b'\x05'] == masked.to_pylist()
+
+ masked_nulls = pa.array(np.array([b'\x05']), type=pa.binary(1),
+ mask=np.array([True]))
+ assert [None] == masked_nulls.to_pylist()
+
+ # Variable Length Binary
+ masked = pa.array(np.array([b'\x05']), type=pa.binary(),
+ mask=np.array([False]))
+ assert [b'\x05'] == masked.to_pylist()
+
+ masked_nulls = pa.array(np.array([b'\x05']), type=pa.binary(),
+ mask=np.array([True]))
+ assert [None] == masked_nulls.to_pylist()
+
+ # Fixed Length Binary, copy
Review comment:
I don't understand what this is supposed to test. The fact that a copy
is made is just an implementation detail.
##########
File path: cpp/src/arrow/python/numpy_to_arrow.cc
##########
@@ -594,7 +594,15 @@ Status NumPyConverter::Visit(const FixedSizeBinaryType&
type) {
if (mask_ != nullptr) {
Ndarray1DIndexer<uint8_t> mask_values(mask_);
- RETURN_NOT_OK(builder.AppendValues(data, length_, mask_values.data()));
+ RETURN_NOT_OK(builder.Reserve(length_));
+ for (int64_t i = 0; i < length_; ++i) {
+ if (mask_values[i]) {
+ RETURN_NOT_OK(builder.AppendNull());
+ } else {
+ RETURN_NOT_OK(builder.Append(data));
+ }
+ data += stride_;
Review comment:
Does this solve the strided conversion case? If so, perhaps you can add
a test for it?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
