Skip to content
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

Update is_list_dtype to handle additional types #250

Merged

Conversation

oliverholworthy
Copy link
Member

Follow-up to #244 . Extending the types supported by is_list_dtype. This wasn't caught in the previous PR because we only ran the NVTabular tests on CPU. There are cases when this function is called by cupy.ndarray and cudf.ListDType values.

The docstring for this function and variable name implied that this function was intended to be used only for series types.

@oliverholworthy oliverholworthy added the chore Maintenance for the repository label Mar 20, 2023
@oliverholworthy oliverholworthy added this to the Merlin 23.03 milestone Mar 20, 2023
@oliverholworthy oliverholworthy self-assigned this Mar 20, 2023
The cudf version does not return the expected type for a cupy list
return cudf_is_list_dtype(ser)
elif cudf and isinstance(ser, cp.ndarray):
return pd.api.types.is_list_like(ser[0])
Copy link
Member Author

Choose a reason for hiding this comment

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

Using pandas is_list_like to check cupy array instead of equivalent in cudf because it doesn't match the pandas version (See: rapidsai/cudf#12975 )

@karlhigley karlhigley merged commit 52358b1 into NVIDIA-Merlin:main Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance for the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants