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

[FEA] Remove redundant parameter in strings::detail::create_chars_child_column #8559

Closed
ttnghia opened this issue Jun 18, 2021 · 0 comments · Fixed by #8576
Closed

[FEA] Remove redundant parameter in strings::detail::create_chars_child_column #8559

ttnghia opened this issue Jun 18, 2021 · 0 comments · Fixed by #8576
Assignees
Labels
feature request New feature or request improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change strings strings issues (C++ and Python)

Comments

@ttnghia
Copy link
Contributor

ttnghia commented Jun 18, 2021

Currently, strings::detail::create_chars_child_column has the following signature:

std::unique_ptr<column> create_chars_child_column(
  size_type strings_count,
  size_type bytes,
  rmm::cuda_stream_view stream        = rmm::cuda_stream_default,
  rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

and its implementation is simply as

std::unique_ptr<column> create_chars_child_column(cudf::size_type strings_count,
                                                  cudf::size_type total_bytes,
                                                  rmm::cuda_stream_view stream,
                                                  rmm::mr::device_memory_resource* mr)
{
  return make_numeric_column(
    data_type{type_id::INT8}, total_bytes, mask_state::UNALLOCATED, stream, mr);
}

Thus, the first parameter is redundant. We need to refactor this to remove that parameter. This is a simple task. However, this API is used in many places thus after refactoring this function we still need to spend some time to fix its usage.

@ttnghia ttnghia added feature request New feature or request good first issue Good for newcomers libcudf Affects libcudf (C++/CUDA) code. strings strings issues (C++ and Python) improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 18, 2021
@davidwendt davidwendt self-assigned this Jun 18, 2021
@davidwendt davidwendt removed the good first issue Good for newcomers label Jun 21, 2021
rapids-bot bot pushed a commit that referenced this issue Jun 22, 2021
…rs_child_column (#8576)

Closes #8559 

The `strings_count` parameter was necessary in a previous version of the `cudf::strings::detail::create_chars_child_column` but has become obsolete. This PR removes the parameter and fixes up the calls to the utility.

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Karthikeyan (https://github.com/karthikeyann)
  - Nghia Truong (https://github.com/ttnghia)

URL: #8576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change strings strings issues (C++ and Python)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants