Skip to content

Add is_nan aliases - #1643

Open
BharatDeva wants to merge 1 commit into
apache:mainfrom
BharatDeva:fix/is-nan-alias-1235
Open

Add is_nan aliases#1643
BharatDeva wants to merge 1 commit into
apache:mainfrom
BharatDeva:fix/is-nan-alias-1235

Conversation

@BharatDeva

Copy link
Copy Markdown
Contributor

Closes #1235

Rationale for this change

The Python API currently exposes is_null() with snake_case naming, but the NaN check is only available as isnan(). Adding is_nan() gives users a casing-consistent spelling while preserving the existing isnan() API.

What changes are included in this PR?

This adds datafusion.functions.is_nan(...) as an alias for isnan(...) and adds Expr.is_nan() as the corresponding expression method. Existing isnan() behavior is unchanged.

The PR also adds coverage for both the function-level alias and the expression method.

Are there any user-facing changes?

Yes. Users can now call functions.is_nan(col(...)) and col(...).is_nan() in addition to the existing isnan spelling. This is additive and does not remove or deprecate the current API.

Testing

uvx pre-commit run --files python/datafusion/functions/__init__.py python/datafusion/expr.py python/tests/test_functions.py python/tests/test_expr.py

Result: passed.

I also ran a WSL canary against the published datafusion==54.0.0 wheel with the modified wrapper files overlaid:

is_nan aliases match isnan output: [False, True, None]

Note: a full Windows uv run pytest is blocked on this machine because building nanoarrow==0.8.0 requires a local C/C++ compiler.

@kosiew kosiew 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.

@BharatDeva
Thanks for working on this!
I took a look and did not find any blocking issues. I have one small suggestion that could make the generated documentation a little clearer.

return Expr(f.isnan(expr.expr))


def is_nan(expr: Expr) -> Expr:

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.

Nice addition! Since is_nan is a direct alias of isnan, it might be helpful to make that relationship a little more obvious in the docstring. A short See Also reference to isnan, or reusing the existing isnan example, would make the generated help a bit easier to navigate.

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.

API Suggestion match casing for isnan and is_null

2 participants