feat: Allow for zero-width fixed size lists #18940

coastalwhite · 2024-09-26T10:03:42Z

This PR properly implements polars.Array(..., 0). This does make a few
fundamental changes to how FixedSizeListArray works internally. Mainly, it
now keeps a length field that actually determines the length instead of
relying on the values.len() / size.

Fixes #18921, #16878.

This PR properly implements `polars.Array(..., 0)`. This does make a few fundamental changes to how `FixedSizeListArray` works internally. Mainly, it now keeps a `length` field that actually determines the length instead of relying the `values.len() / size`. Fixes pola-rs#18921, pola-rs#16878.

coastalwhite · 2024-09-26T13:50:14Z

crates/polars-arrow/src/array/growable/fixed_size_list.rs

-            } else {
-                unreachable!("`GrowableFixedSizeList` expects `DataType::FixedSizeList`")
-            };
+        let size = arrays[0].size();


drive-by: we can already get this size from the array, and the array has the invariant that the dtype must be FSL

crates/polars-arrow/src/array/static_array.rs

coastalwhite · 2024-09-26T13:52:35Z

crates/polars-arrow/src/compute/cast/mod.rs

-        let offsets = list.offsets().buffer().iter();
-        let expected =
-            (list.offsets().first().to_usize()..list.len()).map(|ix| O::from_as_usize(ix * size));
-
-        match offsets
-            .zip(expected)
-            .find(|(actual, expected)| *actual != expected)
-        {
-            Some(_) => polars_bail!(ComputeError:
-                "not all elements have the specified width {size}"
-            ),
-            None => {
-                let sliced_values = list.values().sliced(
-                    list.offsets().first().to_usize(),
-                    list.offsets().range().to_usize(),
-                );
-                cast(sliced_values.as_ref(), inner.dtype(), options)?
-            },
+        let start_offset = list.offsets().first().to_usize();
+        let offsets = list.offsets().buffer();
+
+        let mut is_valid = true;
+        for (i, offset) in offsets.iter().enumerate() {
+            is_valid &= offset.to_usize() == start_offset + i * size;
        }
+
+        polars_ensure!(is_valid, ComputeError: "not all elements have the specified width {size}");
+
+        let sliced_values = list
+            .values()
+            .sliced(start_offset, list.offsets().range().to_usize());
+        cast(sliced_values.as_ref(), inner.dtype(), options)?


drive-by: I feel like this is much clearer

let is_valid = offsets.iter().enumerate().all(|(i, offset)| offset.to_usize() == start_offset + i * size);

coastalwhite · 2024-09-26T13:53:10Z

crates/polars-arrow/src/datatypes/mod.rs

+    pub fn to_fixed_size_list(self, size: usize, is_nullable: bool) -> ArrowDataType {
+        ArrowDataType::FixedSizeList(
+            Box::new(Field::new(
+                PlSmallStr::from_static("item"),
+                self,
+                is_nullable,
+            )),
+            size,
+        )
+    }


added so that we don't constantly have to call .to_physical in reshape.

coastalwhite · 2024-09-26T13:53:42Z

crates/polars-arrow/src/io/ipc/read/array/fixed_size_list.rs

@@ -41,6 +41,7 @@ pub fn read_fixed_size_list<R: Read + Seek>(
    )?;

    let (field, size) = FixedSizeListArray::get_child_and_size(&dtype);
+    polars_ensure!(size > 0, nyi = "Cannot read zero sized arrays from IPC");


I am not sure if this should be implemented, so I just left it like this for now.

coastalwhite · 2024-09-26T13:57:49Z

crates/polars-compute/src/comparisons/array.rs

+        if *self_width == 0 {
+            return Bitmap::new_with_value(false, self.len());
+        }
+


[] != [] = False

coastalwhite · 2024-09-26T13:59:23Z

crates/polars-core/src/frame/column/mod.rs

+    // # Try to Chunked Arrays
+    pub fn try_bool(&self) -> Option<&BooleanChunked> {


drive-by: this allows us to avoid instantiating the errors.

coastalwhite · 2024-09-26T14:00:01Z

crates/polars-core/src/series/from.rs

        ArrowDataType::FixedSizeList(field, size) => {
-            feature_gated!("dtype-array", {


drive-by: This is already feature-gated, no real sense in feature gating it again

coastalwhite · 2024-09-26T14:06:41Z

py-polars/tests/unit/io/test_parquet.py

@@ -924,7 +923,7 @@ def test_parquet_array_dtype_nulls() -> None:
                [[]],
                [[None]],
                [[[None], None]],
-                [[[None], []]],
+                [[[None], [None]]],


This was a clear error

coastalwhite · 2024-09-26T14:07:10Z

py-polars/tests/unit/operations/test_reshape.py

-@pytest.mark.parametrize("shape", [(0, 1), (1, -1), (-1, 1)])
-def test_reshape_empty_invalid_2d(shape: tuple[int, ...]) -> None:
-    s = pl.Series("a", [], dtype=pl.Int64)
-    with pytest.raises(
-        InvalidOperationError,
-        match=re.escape(
-            f"cannot reshape empty array into shape {display_shape(shape)}"
-        ),
-    ):
-        s.reshape(shape)
-


None of the cases now fail

etiennebacher · 2024-09-27T06:54:49Z

Hi, just FYI you need to repeat the "Fixes" for each issue number to automatically close multiple issues. Currently #16878 is not closed ;)

github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars labels Sep 26, 2024

coastalwhite force-pushed the feat/zero-width-arrays branch from 393c918 to 01f6abd Compare September 26, 2024 13:32

coastalwhite force-pushed the feat/zero-width-arrays branch from 01f6abd to 0fa59f9 Compare September 26, 2024 13:33

coastalwhite added 3 commits September 26, 2024 15:47

fix ordering

31c6842

fix type in arrow

fb2340d

remove print in test

010e39f

coastalwhite commented Sep 26, 2024

View reviewed changes

coastalwhite marked this pull request as ready for review September 26, 2024 14:08

coastalwhite requested review from ritchie46, c-peters, alexander-beedie, MarcoGorelli, reswqa and orlp as code owners September 26, 2024 14:08

coastalwhite added 2 commits September 26, 2024 17:03

fix zero reshaping

acbb6d7

fix parquet array statistics

2150be0

ritchie46 approved these changes Sep 27, 2024

View reviewed changes

ritchie46 merged commit a7845c4 into pola-rs:main Sep 27, 2024
26 checks passed

coastalwhite deleted the feat/zero-width-arrays branch September 27, 2024 06:44

This was referenced Sep 27, 2024

Array operation query with filtering results in InvalidOperationError: cannot reshape empty array into shape (-1, 2) #18598

Closed

feat: Allow reshaping empty array to Array dtype #18876

Closed

hexane360 mentioned this pull request Oct 4, 2024

A regression in numpy-to-series conversion #16522

Closed

2 tasks

cmdlineluser mentioned this pull request Oct 5, 2024

Add array function to polars expressions #19079

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Allow for zero-width fixed size lists #18940

feat: Allow for zero-width fixed size lists #18940

coastalwhite commented Sep 26, 2024 •

edited

Loading

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

orlp Sep 26, 2024

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

coastalwhite Sep 26, 2024

etiennebacher commented Sep 27, 2024

		// # Try to Chunked Arrays
		pub fn try_bool(&self) -> Option<&BooleanChunked> {

		ArrowDataType::FixedSizeList(field, size) => {
		feature_gated!("dtype-array", {

feat: Allow for zero-width fixed size lists #18940

feat: Allow for zero-width fixed size lists #18940

Conversation

coastalwhite commented Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etiennebacher commented Sep 27, 2024

coastalwhite commented Sep 26, 2024 •

edited

Loading