-
Notifications
You must be signed in to change notification settings - Fork 915
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
Ensure column.fillna signatures are consistent #14724
Ensure column.fillna signatures are consistent #14724
Conversation
method: Any = None, | ||
dtype: Optional[Dtype] = None, | ||
) -> CategoricalColumn: | ||
method: Optional[str] = None, |
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.
Just checking -- this is non-public so the API break of dropping dtype
is not a "breaking change" right?
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.
I've been working under that assumption that cudf column objects are not public cc @shwina
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.
Yes, that should be fine! Just wanted to check in case this interacts with any future changes you were planning for public types.
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.
Gotcha. Yeah the arguments being modified in Column.fillna
do not clash with Series/DataFrame.fillna
/merge |
Description
Aligns the definitions of
Columns.fillna
among all subclasses.dtype
looks to only needed in certain instances to cast the fill value so can do that separately. Afill_nan
can be avoided with its single usage in acan_cast
routine by checking for nan firstChecklist