Skip to content

fix(engines): disclose categorical pandas fallback - #521

Merged
JohnnyWilson16 merged 8 commits into
FreshCode-Org:mainfrom
WilliamK112:codex/categorical-native-fallback-456
Oct 14, 2026
Merged

JohnnyWilson16 merged 8 commits into
FreshCode-Org:mainfrom
WilliamK112:codex/categorical-native-fallback-456

Conversation

@WilliamK112

@WilliamK112 WilliamK112 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Polars and DuckDB silently skip string cleanup or change the dtype of categorical pandas inputs. Use the existing pre-ingestion pandas fallback for pandas categoricals and Arrow dictionary-backed columns so values, cleaning behavior, category domains, and ordering match the pandas reference. Record the fallback reason, honour warn/error policies before ingestion or pandas work, and report materialized pandas output when a native handle was requested. An explicitly selected pandas engine exits the native-ingest guard without a fallback diagnostic. The fallback matrix and changelog describe the behavior.

Closes #456.

Validation: 34 categorical cases and 83 focused categorical/policy/parity checks pass on pandas 2.3 and pandas 1.5. The original 20 cases failed before implementation. Of 14 added review cases, 12 fail before the Arrow dictionary and explicit-pandas fixes, with two positive controls. The refreshed full fast suite has 7,650 passes, 49 skips, and 94.99% coverage. Its five failures are the reproduced Windows baseline cases (performance-runner timeout, console encoding, generated-code filename escaping, CSV newline round trip, and the zero-second model budget addressed by #520). Ruff, Linux-targeted mypy, and strict MkDocs pass.

Refreshed with current main after #518 merged. All hosted checks pass on the review fixes, including the required quality-fast lane and floating Python 3.9–3.13 matrix.

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 20b7f707-19a5-42ab-ab42-e2e11e7ff0c0
  • 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.

@JohnnyWilson16 JohnnyWilson16 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! The approach is solid, but please address two items before merge:

Arrow-backed categoricals: name == "category" in _ingest.py:51 misses pd.ArrowDtype(pa.dictionary(...)) columns, which Polars still ingests as Categorical and skips text cleanup without disclosure. Expand the condition to:
python
if name == "category" or name.startswith("dictionary"):
engine="pandas" guard: pandas_ingest_fallback_reason() currently returns a fallback reason even when engine="pandas". Please add an early exit at the top:
python
if engine == "pandas":
return None

…ive-fallback-456

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@WilliamK112

WilliamK112 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed both items and merged current main after #518 landed. The guard now includes Arrow dictionary dtypes and exits immediately for engine="pandas". Fourteen added review cases cover ordered/unordered Arrow dictionary parity, warn/error policy, pre-ingestion rejection, and explicit pandas selection; twelve failed before these fixes. All 83 focused tests pass on pandas 1.5 and 2.3. Ruff, mypy, and strict MkDocs pass. The full fast suite has 7,650 passes and 94.99% coverage, with only the five known Windows baseline failures (including the budget boundary fixed in #520).

The docs now describe preserving pandas' actual text-cleaning behavior: pandas currently leaves Arrow dictionary text unchanged, so the fallback preserves that reference behavior and dtype. Hosted CI is running on the updated head before I mark the dependency draft ready.

@WilliamK112
WilliamK112 marked this pull request as ready for review October 13, 2026 12:49
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T12:54:45.740525Z 118bd4f Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 118bd4f54d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/fallback-matrix.md Outdated
@JohnnyWilson16
JohnnyWilson16 merged commit 31afd66 into FreshCode-Org:main Oct 14, 2026
22 checks passed
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.

engine="polars"/"duckdb" skip whitespace stripping and sentinel normalization on categorical columns

2 participants