-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: nw.selectors.by_dtype(nw.Datetime)
was excluding tz-aware for Polars
#1869
fix: nw.selectors.by_dtype(nw.Datetime)
was excluding tz-aware for Polars
#1869
Conversation
narwhals/_polars/namespace.py
Outdated
|
||
native_dtypes: list[pl.DataType | type[pl.DataType]] = [] | ||
for dtype in dtypes: | ||
native_dtype_instantiated = narwhals_to_native_dtype(dtype, self._version) | ||
if isinstance(dtype, type) and issubclass(dtype, DType): | ||
native_dtypes.append(native_dtype_instantiated.__class__) | ||
else: | ||
native_dtypes.append(native_dtype_instantiated) | ||
return PolarsExpr( | ||
pl.selectors.by_dtype( | ||
[narwhals_to_native_dtype(dtype, self._version) for dtype in dtypes] | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue was that we were translating nw.Datetime
to pl.Datetime()
. In other parts of the codebase, that's fine. But here, we need to preserve whether the dtype is instantiated or not, as they behave differently
Like this, we keep the translation from nw.Datetime
to pl.Datetime
closes #1313
What type of PR is this? (check all applicable)
Related issues
Checklist
If you have comments or can explain your changes, please do so below