ronantakizawa/codereview-bench huggingface dataset:
Repository: pola-rs/polars
File: py-polars/tests/unit/series/test_series.py (Python)

Code before:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])

    assert_series_equal(
        s.diff(),
        pl.Series("a", [None, 1, 1, -1, 0, 1, -3]),
    )
    assert_series_equal(
        s.diff(null_behavior="drop"),
        pl.Series("a", [1, 1, -1, 0, 1, -3]),
    )


def test_diff_negative() -> None:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])

    assert_series_equal(
        s.diff(-1),
        pl.Series("a", [-1, -1, 1, 0, -1, 3, None]),
    )
    assert_series_equal(
        s.diff(-1, null_behavior="drop"),
        pl.Series("a", [-1, -1, 1, 0, -1, 3]),
    )


def test_pct_change() -> None:
    s = pl.Series("a", [1, 2, 4, 8, 16, 32, 64])
    expected = pl.Series("a", [None, None, 3.0, 3.0, 3.0, 3.0, 3.0])
    assert_series_equal(s.pct_change(2), expected)
    assert_series_equal(s.pct_change(pl.Series([2])), expected)
    # negative
    assert pl.Series(range(5)).pct_change(-1).to_list() == [
        -1.0,
        -0.5,
        -0.3333333333333333,
        -0.25,
        None,
    ]


def test_skew() -> None:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])

    assert s.skew(bias=True) == pytest.approx(-0.5953924651018018)
    assert s.skew(bias=False) == pytest.approx(-0.7717168360221258)

    df = pl.DataFrame([s])
    assert np.isclose(
        df.select(pl.col("a").skew(bias=False))["a"][0], -0.7717168360221258
    )


Review comment (suggestion):
moved this over to `py-polars/tests/unit/operations/test_pct_change.py`, along with a new test which includes missing values

Code after:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])

    assert_series_equal(
        s.diff(),
        pl.Series("a", [None, 1, 1, -1, 0, 1, -3]),
    )
    assert_series_equal(
        s.diff(null_behavior="drop"),
        pl.Series("a", [1, 1, -1, 0, 1, -3]),
    )


def test_diff_negative() -> None:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])

    assert_series_equal(
        s.diff(-1),
        pl.Series("a", [-1, -1, 1, 0, -1, 3, None]),
    )
    assert_series_equal(
        s.diff(-1, null_behavior="drop"),
        pl.Series("a", [-1, -1, 1, 0, -1, 3]),
    )


def test_skew() -> None:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])

    assert s.skew(bias=True) == pytest.approx(-0.5953924651018018)
    assert s.skew(bias=False) == pytest.approx(-0.7717168360221258)

    df = pl.DataFrame([s])
    assert np.isclose(
        df.select(pl.col("a").skew(bias=False))["a"][0], -0.7717168360221258
    )


def test_kurtosis() -> None:
    s = pl.Series("a", [1, 2, 3, 2, 2, 3, 0])
    expected = -0.6406250000000004

    assert s.kurtosis() == pytest.approx(expected)
    df = pl.DataFrame([s])
    assert np.isclose(df.select(pl.col("a").kurtosis())["a"][0], expected)


def test_sqrt() -> None:
    s = pl.Series("a", [1, 2])
    assert_series_equal(s.sqrt(), pl.Series("a", [1.0, np.sqrt(2)]))
    df = pl.DataFrame([s])
    assert_series_equal(
Repository: pola-rs/polars
File: crates/polars-stream/src/nodes/io_sources/csv.rs (Rust)

Code before:
                break;
            }
        }

        Ok(n_rows_skipped)
    }
}

#[derive(Default)]
struct ChunkReader {
    reader_schema: SchemaRef,
    parse_options: Arc<CsvParseOptions>,
    fields_to_cast: Vec<Field>,
    ignore_errors: bool,
    projection: Vec<usize>,
    null_values: Option<NullValuesCompiled>,
    validate_utf8: bool,
    row_index: Option<RowIndex>,
}

impl ChunkReader {
    fn try_new(
        options: Arc<CsvReadOptions>,
        mut reader_schema: SchemaRef,
        projection: Vec<usize>,
        row_index: Option<RowIndex>,
    ) -> PolarsResult<Self> {
        let mut fields_to_cast: Vec<Field> = options.fields_to_cast.clone();
        prepare_csv_schema(&mut reader_schema, &mut fields_to_cast)?;

        let parse_options = options.parse_options.clone();

        // Logic from `CoreReader::new()`

        let null_values = parse_options
            .null_values
            .clone()
            .map(|nv| nv.compile(&reader_schema))
            .transpose()?;

        let validate_utf8 = matches!(parse_options.encoding, CsvEncoding::Utf8)
            && reader_schema.iter_fields().any(|f| f.dtype().is_string());

        Ok(Self {
            reader_schema,
            parse_options,
            fields_to_cast,
            ignore_errors: options.ignore_errors,
            projection,
            null_values,
            validate_utf8,

Review comment (suggestion):
Removed - `CountLines::find_next()` is updated above to now return the correct count when there are comments


Code after:
                morsel_seq,
            };

            if line_batch_tx.send(batch).await.is_err() {
                break;
            }
        }

        Ok(n_rows_skipped)
    }
}

#[derive(Default)]
struct ChunkReader {
    reader_schema: SchemaRef,
    parse_options: Arc<CsvParseOptions>,
    fields_to_cast: Vec<Field>,
    ignore_errors: bool,
    projection: Vec<usize>,
    null_values: Option<NullValuesCompiled>,
    validate_utf8: bool,
    row_index: Option<RowIndex>,
}

impl ChunkReader {
    fn try_new(
        options: Arc<CsvReadOptions>,
        mut reader_schema: SchemaRef,
        projection: Vec<usize>,
        row_index: Option<RowIndex>,
    ) -> PolarsResult<Self> {
        let mut fields_to_cast: Vec<Field> = options.fields_to_cast.clone();
        prepare_csv_schema(&mut reader_schema, &mut fields_to_cast)?;

        let parse_options = options.parse_options.clone();

        // Logic from `CoreReader::new()`

        let null_values = parse_options
            .null_values
            .clone()
            .map(|nv| nv.compile(&reader_schema))
            .transpose()?;

        let validate_utf8 = matches!(parse_options.encoding, CsvEncoding::Utf8)
            && reader_schema.iter_fields().any(|f| f.dtype().is_string());

        Ok(Self {
            reader_schema,
            parse_options,
            fields_to_cast,
Repository: pola-rs/polars
File: docs/source/src/python/user-guide/expressions/lists.py (Python)

Code before:
import polars as pl

df = pl.DataFrame(
    {
        "names": [
            ["Anne", "Averill"],
            ["Brandon", "Brooke", "Borden", "Branson"],
            ["Camila"],
            ["Dennis", "Doyle", "Dalton"],
        ],
        "children_ages": [
            [5, 7],
            [12, 11, 13, 9],
            [19],
            [8, 11, 18],
        ],
    }
)

print(df)
# --8<-- [end:list-example]

# --8<-- [start:list-aggregation]
result = df.select(
    pl.col("names")
    .list.eval(pl.element().sort_by(pl.col("children_ages"), descending=True))
    .alias("names_by_age"),
    pl.col("children_ages").list.eval(pl.element().min()).alias("min_age"),
    pl.col("children_ages").list.eval(pl.element().max()).alias("max_age"),
)
print(result)
# --8<-- [end:list-aggregation]

# --8<-- [start:list-entropy]
result = df.with_columns(
    pl.col("children_ages").list.eval(pl.element().entropy()).alias("age_entropy"),
)
print(result)
# --8<-- [end:list-entropy]

# --8<-- [start:weather_by_day]
weather_by_day = pl.DataFrame(
    {
        "station": [f"Station {idx}" for idx in range(1, 11)],
        "day_1": [17, 11, 8, 22, 9, 21, 20, 8, 8, 17],
        "day_2": [15, 11, 10, 8, 7, 14, 18, 21, 15, 13],
        "day_3": [16, 15, 24, 24, 8, 23, 19, 23, 16, 10],
    }
)
print(weather_by_day)
# --8<-- [end:weather_by_day]

Review comment (suggestion):
You can't use named columns inside `eval()`

```python
# ComputeError: named columns are not allowed in `eval` functions; consider using `element`
```

Code after:
    {
        "children": [
            [
                {"name": "Anne", "age": 5},
                {"name": "Averill", "age": 7},
            ],
            [
                {"name": "Brandon", "age": 12},
                {"name": "Brooke", "age": 9},
                {"name": "Branson", "age": 11},
            ],
            [{"name": "Camila", "age": 19}],
            [
                {"name": "Dennis", "age": 8},
                {"name": "Doyle", "age": 11},
                {"name": "Dina", "age": 18},
            ],
        ],
    }
)

print(df)
# --8<-- [end:children]

# --8<-- [start:list-sorting]
result = df.select(
    pl.col("children")
    .list.eval(
        pl.element()
        .sort_by(pl.element().struct.field("age"), descending=True)
        .struct.field("name")
    )
    .alias("names_by_age"),
    pl.col("children")
    .list.eval(pl.element().struct.field("age").min())
    .alias("min_age"),
    pl.col("children")
    .list.eval(pl.element().struct.field("age").max())
    .alias("max_age"),
)
print(result)
# --8<-- [end:list-sorting]

# --8<-- [start:list-aggregation]
result = df.select(
    pl.col("children")
    .list.eval(
        pl.element()
        .sort_by(pl.element().struct.field("age"), descending=True)
        .struct.field("name")
    )
Repository: pola-rs/polars
File: crates/polars-core/src/series/proptest.rs (Rust)

Code before:

impl SeriesArbitrarySelection {
    pub fn physical() -> Self {
        Self::BOOLEAN | Self::UINT | Self::INT | Self::FLOAT | Self::STRING | Self::BINARY
    }

    pub fn logical() -> Self {
        Self::TIME
            | Self::DATETIME
            | Self::DATE
            | Self::DURATION
            | Self::DECIMAL
            | Self::CATEGORICAL
            | Self::ENUM
    }

    pub fn nested() -> Self {
        Self::LIST | Self::ARRAY | Self::STRUCT
    }
}

#[derive(Clone)]
pub struct SeriesArbitraryOptions {
    pub allowed_dtypes: SeriesArbitrarySelection,
    pub max_nesting_level: usize,
    pub series_length: usize,
    pub categories_range: RangeInclusive<usize>,
    pub struct_fields_range: RangeInclusive<usize>,
}

impl Default for SeriesArbitraryOptions {
    fn default() -> Self {
        Self {
            allowed_dtypes: SeriesArbitrarySelection::all(),
            max_nesting_level: 3,
            series_length: 5,
            categories_range: 1..=3,
            struct_fields_range: 1..=3,
        }
    }
}

pub fn series_strategy(
    options: Rc<SeriesArbitraryOptions>,
    nesting_level: usize,
) -> impl Strategy<Value = Series> {
    use SeriesArbitrarySelection as S;

    let mut allowed_dtypes = options.allowed_dtypes;

    if options.max_nesting_level <= nesting_level {

Review comment (refactor):
Why did you remove this range argument? I understand when creating `DataFrame`s the columns need to be the same height, but can't you just pass a range with a single length (e.g. `5..6`)?

Doesn't this mean that now the series testing is degraded?

Code after:

impl SeriesArbitrarySelection {
    pub fn physical() -> Self {
        Self::BOOLEAN | Self::UINT | Self::INT | Self::FLOAT | Self::STRING | Self::BINARY
    }

    pub fn logical() -> Self {
        Self::TIME
            | Self::DATETIME
            | Self::DATE
            | Self::DURATION
            | Self::DECIMAL
            | Self::CATEGORICAL
            | Self::ENUM
    }

    pub fn nested() -> Self {
        Self::LIST | Self::ARRAY | Self::STRUCT
    }
}

#[derive(Clone)]
pub struct SeriesArbitraryOptions {
    pub allowed_dtypes: SeriesArbitrarySelection,
    pub max_nesting_level: usize,
    pub series_length_range: RangeInclusive<usize>,
    pub categories_range: RangeInclusive<usize>,
    pub struct_fields_range: RangeInclusive<usize>,
}

impl Default for SeriesArbitraryOptions {
    fn default() -> Self {
        Self {
            allowed_dtypes: SeriesArbitrarySelection::all(),
            max_nesting_level: 3,
            series_length_range: 0..=5,
            categories_range: 0..=3,
            struct_fields_range: 0..=3,
        }
    }
}

pub fn series_strategy(
    options: Rc<SeriesArbitraryOptions>,
    nesting_level: usize,
) -> impl Strategy<Value = Series> {
    use SeriesArbitrarySelection as S;

    let mut allowed_dtypes = options.allowed_dtypes;

    if options.max_nesting_level <= nesting_level {
Repository: pola-rs/polars
File: crates/polars-ops/src/series/ops/index.rs (Rust)

Code before:
use polars_core::error::{PolarsResult, polars_ensure};
use polars_core::prelude::arity::unary_elementwise_values;
use polars_core::prelude::{
    ChunkedArray, DataType, IDX_DTYPE, IdxCa, IdxSize, PolarsIntegerType, Series,
};
use polars_utils::index::ToIdx;

fn convert<T>(ca: &ChunkedArray<T>, target_len: usize) -> PolarsResult<IdxCa>
where
    T: PolarsIntegerType,
    T::Native: ToIdx,
{
    let target_len = target_len as u64;
    Ok(unary_elementwise_values(ca, |v| v.to_idx(target_len)))
}

/// Generic OOB handler on an `IdxCa`.
///
/// - `null_on_oob == false`: error if any index >= len
/// - `null_on_oob == true`: set index >= len to null
fn handle_oob(idx: IdxCa, len: usize, null_on_oob: bool) -> PolarsResult<IdxCa> {
    let len_idx = len as IdxSize;

    if !null_on_oob {
        let oob = idx
            .into_iter()
            .any(|opt_v| opt_v.is_some_and(|v| v >= len_idx));
        polars_ensure!(
            !oob,
            OutOfBounds: "gather indices are out of bounds"
        );
        Ok(idx)
    } else {
        let out: IdxCa = idx
            .into_iter()
            .map(|opt_v| opt_v.and_then(|v| if v >= len_idx { None } else { Some(v) }))
            .collect();
        Ok(out)
    }

Review comment (bug):
This is not your fault, the bug is also in the original code, but I don't think `to_idx` is actually correct. A very negative invalid index could wrap around into a valid positive index, especially if `T` is `i64`.

I'm also not a huge fan of doing two passes over the data, once to convert then to check. I think we can merge all logic and fix the bug by writing `convert` such that it always maps indices that are out-of-bounds based on `target_len` to `null`s.

Then later if `null_on_oob` is true we're immediately done and we only need a check that `in_null_count != out_null_count` for the `null_on_oob = false` case to give an error.

Code after:
use num_traits::ToPrimitive;
use polars_core::error::{PolarsResult, polars_bail, polars_ensure};
use polars_core::prelude::{ChunkedArray, DataType, IdxCa, IdxSize, PolarsIntegerType, Series};

/// UNSIGNED conversion:
///
/// - `0 <= v < target_len`  → `Some(v as IdxSize)`
/// - `v >= target_len`      → `None`
/// - values that cannot be represented as `u64` → `None`
fn convert_unsigned<T>(ca: &ChunkedArray<T>, target_len: usize) -> PolarsResult<IdxCa>
where
    T: PolarsIntegerType,
    T::Native: ToPrimitive,
{
    let len_u64 = target_len as u64;

    let out: IdxCa = ca
        .into_iter()
        .map(|opt_v| {
            opt_v.and_then(|v| {
                // cannot represent as u64 → None (treated as OOB)
                let v_u64 = v.to_u64()?;

                if v_u64 < len_u64 {
                    Some(v_u64 as IdxSize)
                } else {
                    None
                }
            })
        })
        .collect();

    Ok(out)
}

/// SIGNED conversion with Python-style negative semantics:
///
/// - `0 <= v < target_len`          → `Some(v as IdxSize)`
/// - `v >= target_len`              → `None`
Repository: pola-rs/polars
File: py-polars/tests/unit/lazyframe/test_optimizations.py (Python)

Code before:
        .with_row_index("value")
        .with_columns(
            time=pl.lit(dt.date(2026, 1, 1)).dt.combine(pl.col("time")),
            grp=pl.lit(1),
        )
        .lazy()
    )

    # This used to produce out-of-order results.
    # The optimizer previously cleared maintain_order.
    result = (
        df.join_asof(
            targettime,
            left_on="time",
            right_on="targettime",
            strategy="forward",
        )
        .drop_nulls("targettime")
        .group_by("targettime", maintain_order=True)
        .agg(pl.col("value").last())
        .collect()
    )

    # Now should maintain order; just check monotonicity of the targettime key
    times = result["targettime"].to_list()
    assert times == sorted(times)


Review comment (suggestion):
Could we ensure to assert the length of the Series here.

(Further suggestion) I'd prefer we used `assert_frame_equal` to assert equality against a statically declared DataFrame with the expected result values (if it's not too big), up to you.


Code after:
        )
        .with_row_index("value")
        .with_columns(
            time=pl.lit(dt.date(2026, 1, 1)).dt.combine(pl.col("time")),
            grp=pl.lit(1),
        )
        .lazy()
    )

    # This used to produce out-of-order results.
    # The optimizer previously cleared maintain_order.
    q = (
        df.join_asof(
            targettime,
            left_on="time",
            right_on="targettime",
            strategy="forward",
        )
        .drop_nulls("targettime")
        .group_by("targettime", maintain_order=True)
        .agg(pl.col("value").last())
    )

    # Verify optimizer preserves maintain_order on UNIQUE
    plan = q.explain()
    assert "AGGREGATE[maintain_order: true" in plan

    result = q.collect()

    idx_dtype = pl.get_index_type()

    expected = pl.DataFrame(
        {
            "targettime": [
                dt.datetime(2026, 1, 1, 0, 0),
                dt.datetime(2026, 1, 1, 0, 10),
                dt.datetime(2026, 1, 1, 0, 20),
                dt.datetime(2026, 1, 1, 0, 30),
            ],
            "value": pl.Series("value", [0, 600, 1200, 1800], dtype=idx_dtype),
        }
    )

    assert_frame_equal(result, expected)
Repository: pola-rs/polars
File: py-polars/tests/unit/lazyframe/test_optimizations.py (Python)

Code before:
        )
        .with_row_index("value")
        .with_columns(
            time=pl.lit(dt.date(2026, 1, 1)).dt.combine(pl.col("time")),
            grp=pl.lit(1),
        )
        .lazy()
    )

    # This used to produce out-of-order results.
    # The optimizer previously cleared maintain_order.
    result = (
        df.join_asof(
            targettime,
            left_on="time",
            right_on="targettime",
            strategy="forward",
        )
        .drop_nulls("targettime")
        .group_by("targettime", maintain_order=True)
        .agg(pl.col("value").last())
        .collect()
    )

    # Now should maintain order; just check monotonicity of the targettime key
    times = result["targettime"].to_list()
    assert times == sorted(times)


Review comment (suggestion):
```suggestion
    assert result["targettime"].is_sorted()
```

Code after:
            }
        )
        .with_row_index("value")
        .with_columns(
            time=pl.lit(dt.date(2026, 1, 1)).dt.combine(pl.col("time")),
            grp=pl.lit(1),
        )
        .lazy()
    )

    # This used to produce out-of-order results.
    # The optimizer previously cleared maintain_order.
    q = (
        df.join_asof(
            targettime,
            left_on="time",
            right_on="targettime",
            strategy="forward",
        )
        .drop_nulls("targettime")
        .group_by("targettime", maintain_order=True)
        .agg(pl.col("value").last())
    )

    # Verify optimizer preserves maintain_order on UNIQUE
    plan = q.explain()
    assert "AGGREGATE[maintain_order: true" in plan

    result = q.collect()

    idx_dtype = pl.get_index_type()

    expected = pl.DataFrame(
        {
            "targettime": [
                dt.datetime(2026, 1, 1, 0, 0),
                dt.datetime(2026, 1, 1, 0, 10),
                dt.datetime(2026, 1, 1, 0, 20),
                dt.datetime(2026, 1, 1, 0, 30),
            ],
            "value": pl.Series("value", [0, 600, 1200, 1800], dtype=idx_dtype),
        }
    )

    assert_frame_equal(result, expected)
Repository: pola-rs/polars
File: crates/polars-io/src/lib.rs (Rust)

Code before:
pub mod hive;

pub fn get_upload_chunk_size() -> usize {
    use std::sync::LazyLock;

    return *UPLOAD_CHUNK_SIZE;

    static UPLOAD_CHUNK_SIZE: LazyLock<usize> = LazyLock::new(|| {
        let v = std::env::var("POLARS_UPLOAD_CHUNK_SIZE")
            .map(|x| {
                x.parse::<usize>()
                    .ok()
                    .filter(|x| *x > 0)
                    .unwrap_or_else(|| panic!("invalid value for POLARS_UPLOAD_CHUNK_SIZE: {x}"))
            })
            .unwrap_or(64 * 1024 * 1024);

        if polars_core::config::verbose() {
            eprintln!("async upload_chunk_size: {v}")
        }

        v
    });
}

pub fn get_upload_max_concurrency() -> usize {
    use std::sync::LazyLock;

    return *UPLOAD_MAX_CONCURRENCY;

    static UPLOAD_MAX_CONCURRENCY: LazyLock<usize> = LazyLock::new(|| {
        // Max number of parts concurrently uploaded per Writer.
        // @NOTE. The object_store::BufWriter uses 8 as default.
        let v = std::env::var("POLARS_UPLOAD_MAX_CONCURRENCY")
            .map(|x| {
                x.parse::<usize>()
                    .ok()
                    .filter(|x| *x > 0)
                    .unwrap_or_else(|| {
                        panic!("invalid value for POLARS_UPLOAD_MAX_CONCURRENCY: {x}")
                    })
            })
            .unwrap_or(8);

        if polars_core::config::verbose() {
            eprintln!("async upload_max_concurrency: {v}")
        }

        v
    });
}

Review comment (suggestion):
nit -
```suggestion
pub fn get_upload_concurrency() -> usize {
```

Code after:
pub mod hive;

pub fn get_upload_chunk_size() -> usize {
    use std::sync::LazyLock;

    return *UPLOAD_CHUNK_SIZE;

    static UPLOAD_CHUNK_SIZE: LazyLock<usize> = LazyLock::new(|| {
        let v = std::env::var("POLARS_UPLOAD_CHUNK_SIZE")
            .map(|x| {
                x.parse::<usize>()
                    .ok()
                    .filter(|x| *x > 0)
                    .unwrap_or_else(|| panic!("invalid value for POLARS_UPLOAD_CHUNK_SIZE: {x}"))
            })
            .unwrap_or(64 * 1024 * 1024);

        if polars_core::config::verbose() {
            eprintln!("async upload_chunk_size: {v}")
        }

        v
    });
}

pub fn get_upload_concurrency() -> usize {
    use std::sync::LazyLock;

    return *UPLOAD_CONCURRENCY;

    static UPLOAD_CONCURRENCY: LazyLock<usize> = LazyLock::new(|| {
        // Max number of parts concurrently uploaded per Writer.
        // @NOTE. The object_store::BufWriter uses 8 as default.
        let v = std::env::var("POLARS_UPLOAD_CONCURRENCY")
            .map(|x| {
                x.parse::<usize>()
                    .ok()
                    .filter(|x| *x > 0)
                    .unwrap_or_else(|| panic!("invalid value for POLARS_UPLOAD_CONCURRENCY: {x}"))
            })
            .unwrap_or(8);

        if polars_core::config::verbose() {
            eprintln!("async upload_concurrency: {v}")
        }

        v
    });
}