Skip to content

MAINT cleanup utils.__init__: move nan utils into dedicated submodule #28495

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

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

jeremiedbb
Copy link
Member

Extracted from #26686 to ease the reviews. The end goal is to clean the utils.init.py module as explained in the linked PR.

I didn't find a relevant already existing module to move is_scalar_nan and _is_pandas_na into so I created a dedicated utils._missing submodule. Maybe it could go into vaildation since it's kind of related to input/data validation but I not sure if it's the right place, let me know what you think.

Since both function are not listed in classes.rst, they're assumed private so I made them only importable from utils._missing.

I also took the opportunity to rename _is_pandas_na into is_pandas_na since there's no need to multiply the leading underscores: it already comes from a private module.

@jeremiedbb jeremiedbb added No Changelog Needed Quick Review For PRs that are quick to review labels Feb 21, 2024
Copy link

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: d1339b7. Link to the linter CI: here

Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @jeremiedbb

Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasjpfan thomasjpfan merged commit e94eae4 into scikit-learn:main Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Needed Quick Review For PRs that are quick to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants