Skip to content

Conversation

@aucahuasi
Copy link
Contributor

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@aucahuasi aucahuasi changed the title [C++] ARROW-14035: Implement count distinct kernel ARROW-14035: [C++] Implement count distinct kernel Sep 28, 2021
@github-actions
Copy link

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

I realize this is very much a draft, but I left some notes.

@ianmcook
Copy link
Member

@aucahuasi may I push a commit to this PR to implement the R binding? (ARROW-14036)

@aucahuasi
Copy link
Contributor Author

@ianmcook sure!

@aucahuasi
Copy link
Contributor Author

@aucahuasi aucahuasi changed the title ARROW-14035: [C++] Implement count distinct kernel ARROW-14035: [C++][Python] Implement count distinct kernel Sep 29, 2021
@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch from 7765ce6 to 8d61f6a Compare September 29, 2021 00:18
@aucahuasi
Copy link
Contributor Author

@lidavidm @edponce Once again, thank you for the feedback!
I sent new changes and I think this PR is ready for review.
btw I also added a test in python with timestamps.

@aucahuasi aucahuasi marked this pull request as ready for review September 29, 2021 00:20
@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch from 8d61f6a to 5570299 Compare September 29, 2021 03:09
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. I'm adding some comments to Eduardo's.

@aucahuasi aucahuasi changed the title ARROW-14035: [C++][Python] Implement count distinct kernel ARROW-14035: [C++][Python][R] Implement count distinct kernel Sep 29, 2021
@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch from 5570299 to 47571c7 Compare September 30, 2021 18:14
@aucahuasi aucahuasi requested a review from pitrou September 30, 2021 18:18
@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch from 47571c7 to e6c7d1d Compare October 1, 2021 15:19
@aucahuasi
Copy link
Contributor Author

Let me know about any additional feedback please!
Thanks!
cc @pitrou @ianmcook

@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch 3 times, most recently from efe7974 to 5711dfd Compare October 2, 2021 21:02
@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch from cc10c5f to a1b6dbb Compare October 3, 2021 20:58
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Thanks for the update @aucahuasi . It looks like there's a couple issues remaining, see below.

@aucahuasi aucahuasi force-pushed the ARROW-14035-implement-count_distinct-kernel branch 3 times, most recently from e0c42bb to 5c47cda Compare October 4, 2021 17:23
@aucahuasi aucahuasi requested a review from pitrou October 4, 2021 17:36
aucahuasi and others added 3 commits October 5, 2021 14:43
finish basic implementation & add tests and docs

add count_distinct_doc

add R binding for count_distinct kernel

support more types & add more tests

minor change

reduce code

use type matchers to reduce even more code

minor change

Update docs/source/cpp/compute.rst Co-authored-by: Ian Cook <ianmcook@gmail.com>

minor changes

add tests for scalar types and fix an issue for scalars
@pitrou pitrou force-pushed the ARROW-14035-implement-count_distinct-kernel branch from 5c47cda to 5de1a5c Compare October 5, 2021 13:10
ViniciusSouzaRoque pushed a commit to s1mbi0se/arrow that referenced this pull request Oct 20, 2021
https://issues.apache.org/jira/browse/ARROW-14035

Closes apache#11257 from aucahuasi/ARROW-14035-implement-count_distinct-kernel

Lead-authored-by: Percy Camilo Triveño Aucahuasi <percy.camilo.ta@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Neal Richardson <neal.p.richardson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants