[python-package] Add categorical support for Polars/Arrow via Narwhals - #7343
[python-package] Add categorical support for Polars/Arrow via Narwhals#7343maxzw wants to merge 51 commits into
Conversation
0832e6a to
f2cbf5e
Compare
b325341 to
f1fc144
Compare
Unknown dtype
narwhals-dev/narwhals#3719
6b9a7f6 to
0048164
Compare
b1e39f7 to
3a356aa
Compare
|
Absolutely! Done now! |
04b92d7 to
7b31f8c
Compare
7b31f8c to
78a045c
Compare
4a02fb2 to
626df36
Compare
|
hey thanks for keeping this updated @maxzw . I'm going to cancel the latest CI run...I'm about to start the v4.7.0 release and I want to be sure the remaining jobs get priority in scheduling. I'll restart it for you here when that's done. Sorry for the brief disruption. |
867e5ca to
634ad93
Compare
|
Hi, quick status update on this PR. I've spent more time investigating how the Polars global string cache issue I described above can be handled uniformly across dataframe backends, and how ordered vs unordered categoricals interact with the C++ bin mapper. The current state: I've split out a final prerequisite PR (#7376) that adds pandas tests documenting the expected categorical encoding behavior ( Once #7376 is in I'll resolve conflicts and mark it ready for proper review. Apologies for continuing to push commits on an open PR, I understand that makes it hard to review a moving target. |
634ad93 to
a69745a
Compare
|
@jameslamb with and #7376 merged and the merge conflicts resolved, I've converged on the current implementation and am stepping back to allow time for a proper review. Categorical support for Polars and Arrow is (to the best of my knowledge) fully aligned with pandas, except for Polars' unordered categorical ( Why we use
|
|
Just to document; In the future, |
borchero
left a comment
There was a problem hiding this comment.
Sorry for the late review 😅 I have left a few preliminary comments
e77390a to
d7e94d7
Compare
Add categorical support for Polars/Arrow via Narwhals
This PR is a follow up of #7275 (cc @borchero), contributes to #6204 and replaces #7264.
Motivation
Now that DataFrame inputs are routed through narwhals, it seemed like a small step to also pick up categorical columns from polars the same way we already do for pandas. Previously,
pl.Categorical/pl.Enumcolumns were silently ignored, which meant they'd either fail downstream or get mis-encoded.What Changed?
The pandas-specific
_data_from_pandashelper has been replaced with_data_from_narwhals, which detects and encodes categoricals via narwhals. The pandas fast path (_pandas_to_numpy) is preserved for the no-copy case. Thepandas_categoricalattribute name is kept as-is to stay compatible with saved models (an issue was opened to rename this in the future: #7361).Added generic dtype validation in the Narwhals dataframe path with
_check_for_bad_narwhals_dtypesso LightGBM now explicitly accepts only integer, float, bool and categorical columns across supported backends. I consider this in scope because the old pandas path had implicit dtype validation via pandas_to_numpy conversions, and the new narwhals path needs an explicit equivalent to preserve the same user-facing error behavior.New tests added for polars and arrow based on #7359 and #7376. Currently, only pandas, polars and pyarrow are covered by categorical tests, so those are the only backends I'd claim as supported for now. Other narwhals backends (e.g. modin, cudf, dask) should benefit too in principle - I haven't tested that.