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

The example in the documentation for get_dremel_data() seems incorrect at line#1764 #11396

Closed
hyperbolic2346 opened this issue Jul 28, 2022 · 3 comments · Fixed by #17242
Closed
Labels
cuIO cuIO issue doc Documentation libcudf Affects libcudf (C++/CUDA) code.

Comments

@hyperbolic2346
Copy link
Contributor

I know this isn't part of the review, but the example in the documentation for get_dremel_data() seems incorrect at line#1738 (now #1764):

 * Given a LIST column of type `List<List<int>>` like so:
 * ```
 * col = {
 *    [],
 *    [[], [1, 2, 3], [4, 5]],
 *    [[]]
 * }
 * ```
 * We can represent it in cudf format with two level of offsets like this:
 * ```
 * Level 0 offsets = {0, 0, 3, 5, 6}
 * Level 1 offsets = {0, 0, 3, 5, 5}
 * Values          = {1, 2, 3, 4, 5}
 * ```

The Level 0 offset values can't exceed 4, since Level 1 has only 4 ranges (i.e. 5 offsets).
I'll try make sense of this at a later date, but I'm not sure I could follow along.

Originally posted by @mythrocks in #11328 (comment)

@vyasr
Copy link
Contributor

vyasr commented Jul 29, 2022

@bdice noticed the same issues while reviewing #11129, which moves these dremel files around but also doesn't modify them. We'll want to do a thorough pass through both the docstring of get_dremel_data and the actual function implementation once both #11129 and of @hyperbolic2346's Parquet-related PRs are merged where we just focus on cleaning up that functionality and documentation.

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@hyperbolic2346 hyperbolic2346 added the good first issue Good for newcomers label Aug 29, 2022
@GregoryKimball GregoryKimball added doc Documentation cuIO cuIO issue and removed good first issue Good for newcomers labels Nov 21, 2022
@GregoryKimball GregoryKimball added the libcudf Affects libcudf (C++/CUDA) code. label Apr 2, 2023
@vyasr
Copy link
Contributor

vyasr commented May 14, 2024

@bdice @hyperbolic2346 is this something you two want to take another look at? I think it got lost in the shuffle of the other PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuIO cuIO issue doc Documentation libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants