jorisvandenbossche commented on code in PR #40160:
URL: https://github.com/apache/arrow/pull/40160#discussion_r1507745339


##########
python/pyarrow/tests/test_array.py:
##########
@@ -3635,12 +3651,139 @@ def test_list_view_from_arrays(list_array_type):
     assert array.sizes.to_pylist() == sizes
 
 
[email protected](('list_array_type'),
-                         [pa.ListViewArray, pa.LargeListViewArray])
-def test_list_view_flatten(list_array_type):
[email protected](('list_array_type', 'list_type_factory'),
+                         [(pa.ListViewArray, pa.list_view),
+                          (pa.LargeListViewArray, pa.large_list_view)])
+def test_list_view_from_arrays_fails(list_array_type, list_type_factory):
+    values = [1, 2]
+    offsets = [0, 1, None]
+    sizes = [1, 1, 0]
+    mask = pa.array([False, False, True])
+
+    # Ambiguous to specify both validity map and offsets or sizes with nulls
+    with pytest.raises(pa.lib.ArrowInvalid):
+        list_array_type.from_arrays(offsets, sizes, values, mask=mask)
+
+    offsets = [0, 1, 1]
+    array = list_array_type.from_arrays(offsets, sizes, values, mask=mask)
+    array_slice = array[1:]
+
+    # List offsets and sizes must not be slices if a validity map is specified
+    with pytest.raises(pa.lib.ArrowInvalid):
+        list_array_type.from_arrays(
+            array_slice.offsets, array_slice.sizes,
+            array_slice.values, mask=array_slice.is_null())
+
+
[email protected](('list_array_type', 'list_type_factory'), (
+    (pa.ListViewArray, pa.list_view),
+    (pa.LargeListViewArray, pa.large_list_view)
+))
[email protected]("arr", (
+    [None, [0]],
+    [None, [0, None], [0]],
+    [[0], [1]],
+))
+def test_list_view_from_factory(
+    list_array_type, list_type_factory, arr
+):
+    arr = pa.array(arr, type=list_type_factory(pa.int8()))
+    reconstructed_arr = list_array_type.from_arrays(
+        arr.offsets, arr.sizes, arr.values, mask=arr.is_null())

Review Comment:
   ```suggestion
       result = pa.array(arr, type=list_type_factory(pa.int8()))
       assert result.to_pylist() == arr
   ```
   
   Maybe something like this instead? Because right now it doesn't really 
guarantee it was constructed correctly



##########
python/pyarrow/tests/test_scalars.py:
##########
@@ -565,26 +563,38 @@ def test_list(ty, klass):
         s[2]
 
 
-def test_list_from_numpy():
-    s = pa.scalar(np.array([1, 2, 3], dtype=np.int64()))
-    assert s.type == pa.list_(pa.int64())
[email protected]('ty', [
+    pa.list_(pa.int64()),
+    pa.large_list(pa.int64()),
+    pa.list_view(pa.int64()),
+    pa.large_list_view(pa.int64())
+])
+def test_list_from_numpy(ty):
+    s = pa.scalar(np.array([1, 2, 3], dtype=np.int64()), type=ty)

Review Comment:
   Can you keep the case where type is None (inferred)? Eg add None to the 
parametrzation, and then after this line set `ty` to the default inferred one 
if it was None



##########
python/pyarrow/tests/test_array.py:
##########
@@ -3635,12 +3651,139 @@ def test_list_view_from_arrays(list_array_type):
     assert array.sizes.to_pylist() == sizes
 
 
[email protected](('list_array_type'),
-                         [pa.ListViewArray, pa.LargeListViewArray])
-def test_list_view_flatten(list_array_type):
[email protected](('list_array_type', 'list_type_factory'),
+                         [(pa.ListViewArray, pa.list_view),
+                          (pa.LargeListViewArray, pa.large_list_view)])
+def test_list_view_from_arrays_fails(list_array_type, list_type_factory):
+    values = [1, 2]
+    offsets = [0, 1, None]
+    sizes = [1, 1, 0]
+    mask = pa.array([False, False, True])
+
+    # Ambiguous to specify both validity map and offsets or sizes with nulls
+    with pytest.raises(pa.lib.ArrowInvalid):
+        list_array_type.from_arrays(offsets, sizes, values, mask=mask)
+
+    offsets = [0, 1, 1]
+    array = list_array_type.from_arrays(offsets, sizes, values, mask=mask)
+    array_slice = array[1:]
+
+    # List offsets and sizes must not be slices if a validity map is specified
+    with pytest.raises(pa.lib.ArrowInvalid):
+        list_array_type.from_arrays(
+            array_slice.offsets, array_slice.sizes,
+            array_slice.values, mask=array_slice.is_null())
+
+
[email protected](('list_array_type', 'list_type_factory'), (
+    (pa.ListViewArray, pa.list_view),
+    (pa.LargeListViewArray, pa.large_list_view)
+))
[email protected]("arr", (
+    [None, [0]],
+    [None, [0, None], [0]],
+    [[0], [1]],
+))
+def test_list_view_from_factory(
+    list_array_type, list_type_factory, arr
+):
+    arr = pa.array(arr, type=list_type_factory(pa.int8()))
+    reconstructed_arr = list_array_type.from_arrays(
+        arr.offsets, arr.sizes, arr.values, mask=arr.is_null())

Review Comment:
   Although this test might also not add really value compared to what is 
already tested in test_convert_builtin.py?



-- 
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]

Reply via email to