Skip to content

fix: support non-float polars columns in is_nan_or_none - #297

Open
Raashish Aggarwal (raashish1601) wants to merge 2 commits into
Nixtla:mainfrom
raashish1601:fix/is-nan-or-none-polars-strings
Open

Raashish Aggarwal (raashish1601) wants to merge 2 commits into
Nixtla:mainfrom
raashish1601:fix/is-nan-or-none-polars-strings

Conversation

@raashish1601

Copy link
Copy Markdown

is_nan_or_none fails on polars columns that aren't numeric, because polars only supports is_nan on numeric dtypes:

import polars as pl
from utilsforecast.processing import is_nan_or_none
is_nan_or_none(pl.Series(["a", None]))
# polars.exceptions.InvalidOperationError: `is_nan` operation not supported for dtype `str`

This shows up in neuralforecast when fitting on a polars frame with a string categorical exogenous column: _check_nan calls is_nan_or_none on every column (see Nixtla/neuralforecast#1634, where it was suggested to fix it here).

For polars series that are not floats, is_nan_or_none now returns is_null(), since only float columns can hold NaN. Float columns and pandas are unchanged.

Extended test_is_nan_or_none with polars string, boolean, integer and categorical series containing a null. It fails on main and passes with this change. tests/test_processing.py passes apart from three polars concat/backtest tests that also fail on main with the polars version I have locally (2.0, above the <=1.31 pin).

Comment thread utilsforecast/processing.py Outdated


def is_nan_or_none(s: Series) -> Series:
if isinstance(s, pl_Series) and not s.dtype.is_float():

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.

please move this to the is_nan function. in the polars branch we can check if its float and call is_nan and otherwise return a series filled with false

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in a4c1d45. is_nan now calls is_nan only for polars float columns and returns an all-false series otherwise, and is_nan_or_none is back to is_nan(s) | is_none(s).

Comment thread tests/test_processing.py Outdated
is_nan_or_none(pl.Series([np.nan, 1.0, None])).to_numpy(),
np.array([True, False, True]),
)
for values in (["a", None, "b"], [True, None, False], [1, None, 2]):

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.

please move these to the is_nan test

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved them to test_is_nan in a4c1d45. They check is_nan on string, boolean, integer and categorical series, and is_nan_or_none on the same series.

This branch has not been deployed

No deployments
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.

2 participants