Skip to content

fix: return null for all-null rows in mean_horizontal (#4056) - #4060

Closed
ioseba wants to merge 1 commit into
narwhals-dev:mainfrom
ioseba:fix-4056-mean-horizontal-all-null
Closed

ioseba wants to merge 1 commit into
narwhals-dev:mainfrom
ioseba:fix-4056-mean-horizontal-all-null

Conversation

@ioseba

@ioseba ioseba commented Oct 11, 2026 •

Copy link
Copy Markdown

Description

For rows where every input column is null, mean_horizontal previously divided 0 / 0, returning NaN instead of null on PyArrow, DuckDB, and pandas[pyarrow]. Because NaN is not considered null on PyArrow and DuckDB, is_null() unexpectedly evaluated to False.

This PR:

  • In _arrow/namespace.py: uses pc.if_else to return scalar null when non-null count is 0.
  • In _duckdb/namespace.py: uses when(count > 0, total / count).otherwise(lit(None)).
  • In _pandas_like/namespace.py: uses .zip_with(count > 0, None) to set None/<NA> when count is 0, resolving the issue for pandas[pyarrow] as well.
  • Adds regression unit tests in tests/expr_and_series/mean_horizontal_test.py covering all constructors and duckdb.

What type of PR is this? (check all applicable)

  • 🐛 Bug Fix
  • ✅ Test

Related issues

AI assistance

  • AI tools were used.

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

Summary by CodeRabbit

  • Bug Fixes
    • Horizontal means now return null when all input values in a row are null, consistently across supported backends.
    • Rows with a mix of null and non-null values continue to average only the non-null values.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c202684a-4393-4388-af49-db4634ff69c7

📥 Commits

Reviewing files that changed from the base of the PR and between 4d6f7b0 and 31e5072.


📒 Files selected for processing (4)
  • src/narwhals/_arrow/namespace.py
  • src/narwhals/_duckdb/namespace.py
  • src/narwhals/_pandas_like/namespace.py
  • tests/expr_and_series/mean_horizontal_test.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.



📝 Walkthrough

Walkthrough

Horizontal mean implementations in Arrow, DuckDB, and pandas-like backends now return null for rows where all inputs are null. Tests cover these rows and rows with non-null values.

Changes

Horizontal Mean Null Handling

Layer / File(s) Summary
Guard horizontal means against zero counts
src/narwhals/_arrow/namespace.py, src/narwhals/_duckdb/namespace.py, src/narwhals/_pandas_like/namespace.py
Each backend now returns null when a row has no non-null inputs.
Test horizontal mean results
tests/expr_and_series/mean_horizontal_test.py
Tests cover all-null rows and rows with values for a constructor-backed dataframe and a DuckDB lazy dataframe. The DuckDB test skips when DuckDB is unavailable.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: fbruzzesi


Merge Risk | ⚪ Minimal · up to 31e50

Merge Risk: ⚪ Minimal · up to 31e50

The change is mergeable after normal checks; no actionable risk has been identified.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 31e50

The change is limited to numerical result handling and regression tests. It preserves existing inputs and execution paths, with no identified expansion of access, privileges, or security-sensitive behavior.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The supported exposure is a result-semantics change for callers already able to evaluate horizontal means on these backends. The inspected changes do not expand the set of evaluated input expressions or introduce a new privileged operation.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main bug fix: returning null for all-null rows in mean_horizontal.
Description check Passed The description explains the bug, identifies the affected backends, summarizes the implementation, references the related issue, and completes the relevant checklist items.
Linked Issues check Passed Issue #4056 requires a null result and is_null() == True when every input is null across pandas, Polars, PyArrow, and DuckDB. The PR masks zero-count rows in ArrowNamespace.mean_horizontal, `DuckD…
Out of Scope Changes check Passed The changes stay within issue #4056. The namespace changes implement zero-count null handling, and the added tests verify the reported regression and preserve valid means. No unrelated production or t…

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@FBruzzesi

Copy link
Copy Markdown
Member

Hi @ioseba thanks for the contribution and welcome to Narwhals. Sadly this is a duplicate of #4059 .
I hope to see you keep contributing in the future, focusing on issues that are still open and no one is working on them

@ioseba

ioseba commented Oct 11, 2026

Copy link
Copy Markdown
Author

Thanks for the heads up and the warm welcome @FBruzzesi!

My apologies, I hadn't seen the other PR when submitting. Totally understand, no worries at all! I will keep an eye on open issues that need a hand to contribute to soon. Keep up the great work with Narwhals! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: mean_horizontal returns NaN instead of null for all-null rows on PyArrow and DuckDB

2 participants