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.

Sign up for GitHub

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

Jump to bottom

Add to/from arrow for fixed point columns #7609

Merged
merged 48 commits into from
Apr 12, 2021
Merged

Add to/from arrow for fixed point columns #7609

merged 48 commits into from
Apr 12, 2021

Conversation

shwina
Copy link
Contributor

@shwina shwina commented Mar 16, 2021

Closes #7116

@codereport codereport added 2 - In Progress Currently a work in progress feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Mar 16, 2021
@github-actions github-actions bot added the CMake CMake build issue label Mar 17, 2021
@codereport
Copy link
Contributor

rerun tests

@jrhemstad
Copy link
Contributor

is only failing on CUDA 10.1 and it is due to having changed the file from .cpp to .cu. Jake mentioned that this sounds like an NVCC bug.

What should we do for now? @kkraus14 @jrhemstad

@kkraus14 Can this feature wait until 0.20 where we drop 10.1?

@kkraus14
Copy link
Collaborator

@kkraus14 Can this feature wait until 0.20 where we drop 10.1?

@sameerz is that okay on your side?

vuule
vuule requested changes Mar 31, 2021
View reviewed changes
Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

Looks good, couple of minor suggestions.

cpp/src/interop/from_arrow.cu Outdated Show resolved Hide resolved
cpp/src/interop/from_arrow.cu Outdated Show resolved Hide resolved
cpp/src/interop/from_arrow.cu Outdated Show resolved Hide resolved
@sameerz
Copy link
Contributor

sameerz commented Mar 31, 2021

@kkraus14 Can this feature wait until 0.20 where we drop 10.1?

@sameerz is that okay on your side?

@kkraus14 Yes we can wait until 0.20.

@kkraus14 kkraus14 changed the base branch from branch-0.19 to branch-0.20 March 31, 2021 21:17
@kkraus14
Copy link
Collaborator

Updated target to branch-0.20

@codereport codereport requested a review from vuule March 31, 2021 21:49
vuule
vuule approved these changes Mar 31, 2021
View reviewed changes
cpp/src/interop/from_arrow.cu Outdated Show resolved Hide resolved
harrism
harrism previously requested changes Mar 31, 2021
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Looks good, just a request to isolate the .cu-requiring code.

cpp/CMakeLists.txt Show resolved Hide resolved
cpp/src/interop/from_arrow.cu Outdated Show resolved Hide resolved
@harrism harrism dismissed their stale review April 7, 2021 22:27

Resolved

@codereport codereport requested a review from harrism April 8, 2021 14:11
@codereport
Copy link
Contributor

@harrism Are you able to approve? looks like you completed a review. We can merge once he have the last approval.

@codereport
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f593e34 into rapidsai:branch-0.20 Apr 12, 2021
devavret pushed a commit to harrism/cudf that referenced this pull request Apr 14, 2021
@shwina @devavret
Add to/from arrow for fixed point columns (rapidsai#7609)
e54bffd 
Closes rapidsai#7116

Authors:
  - Ashwin Srinath (https://github.com/shwina)
  - Conor Hoekstra (https://github.com/codereport)

Approvers:
  - Keith Kraus (https://github.com/kkraus14)
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Vukasin Milovanovic (https://github.com/vuule)
  - Jake Hemstad (https://github.com/jrhemstad)

URL: rapidsai#7609
@shwina shwina mentioned this pull request Jun 2, 2021
Closed
@codereport codereport mentioned this pull request Aug 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codereport codereport codereport left review comments

@vuule vuule vuule approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@kkraus14 kkraus14 kkraus14 approved these changes

@brandon-b-miller brandon-b-miller Awaiting requested review from brandon-b-miller

@harrism harrism Awaiting requested review from harrism

Assignees

@shwina shwina

@codereport codereport

Labels
4 - Needs Review Waiting for reviewer to review or respond CMake CMake build issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Fixed-point support for to_arrow and from_arrow
8 participants
@shwina @codereport @galipremsagar @jrhemstad @kkraus14 @sameerz @harrism @vuule