Repository navigation
fix(engines): disclose categorical pandas fallback - #521
JohnnyWilson16 merged 8 commits into
Conversation
Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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. Comment |
JohnnyWilson16
left a comment
There was a problem hiding this comment.
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>
|
Addressed both items and merged current main after #518 landed. The guard now includes Arrow dictionary dtypes and exits immediately for 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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 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".
Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
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.