Implement `extract_datetime_component` in `libcudf`/`pylibcudf` #16776

brandon-b-miller · 2024-09-09T19:57:00Z

cpp/include/cudf/datetime.hpp

bdice · 2024-09-09T21:23:14Z

cpp/src/datetime/datetime_ops.cu

+{
+  CUDF_FUNC_RANGE();
+  switch (component) {
+    case datetime_component::YEAR: return extract_year(column, mr);


If we plan to deprecate functions like extract_year, we should reverse the call stack here.

This function should dispatch to return detail::apply_datetime_op<detail::extract_component_operator<...>, cudf::type_id::INT16>(column, stream, mr); based on the enum, and we should make extract_year call extract_datetime_component(column, datetime_component::YEAR, mr);.

Made these changes with the latest commits - let me know if this is along the lines of what you meant.

cpp/src/datetime/datetime_ops.cu

python/pylibcudf/pylibcudf/datetime.pyx

cpp/include/cudf/datetime.hpp

cpp/src/datetime/datetime_ops.cu

python/pylibcudf/pylibcudf/datetime.pyx

wence- · 2024-09-10T16:14:02Z

python/pylibcudf/pylibcudf/libcudf/datetime.pxd

 from libcpp.memory cimport unique_ptr
 from pylibcudf.libcudf.column.column cimport column
 from pylibcudf.libcudf.column.column_view cimport column_view
 from pylibcudf.libcudf.scalar.scalar cimport scalar


 cdef extern from "cudf/datetime.hpp" namespace "cudf::datetime" nogil:
+    cpdef enum class datetime_component(int32_t):


As @bdice suggests, make the underlying type explicit in the C++ and then match it here.

wence- · 2024-09-10T16:15:06Z

python/pylibcudf/pylibcudf/tests/test_datetime.py

+        pa.scalar(1694004645123456789, pa.timestamp("ns")),
+        pa.scalar(1544024645123456789, pa.timestamp("ns")),
+        pa.scalar(1682342345346235434, pa.timestamp("ns")),
+        pa.scalar(1445624625623452452, pa.timestamp("ns")),


Is there a reason we can't use conversion from datetime? It's also not obvious to me that these are the same timepoints as before.

Co-authored-by: Bradley Dice <[email protected]> Co-authored-by: Lawrence Mitchell <[email protected]>

cpp/src/datetime/datetime_ops.cu

wence-

We can do a little tidy-up in the implementation on the C++ side, I think.

cpp/src/datetime/datetime_ops.cu

brandon-b-miller · 2024-10-02T17:23:24Z

I'm having a little trouble figuring out why this warning/error is being generated during the docs build:

WARNING: cpp:func reference target not found: cudf::datetime::extract_datetime_component

Can anyone spot what I've got wrong? Maybe @bdice

bdice · 2024-10-02T18:14:52Z

I'm having a little trouble figuring out why this warning/error is being generated during the docs build:
WARNING: cpp:func reference target not found: cudf::datetime::extract_datetime_component
Can anyone spot what I've got wrong? Maybe @bdice

I'm not sure. Maybe try :cpp:func:`extract_datetime_component`.

Matt711 · 2024-10-02T18:36:55Z

I'm having a little trouble figuring out why this warning/error is being generated during the docs build:
WARNING: cpp:func reference target not found: cudf::datetime::extract_datetime_component
Can anyone spot what I've got wrong? Maybe @bdice

I built the docs locally and extract_datetime_component does not show up in the libcudf API docs.

Edit: It should be in this part of the docs, but it isn't

Matt711 · 2024-10-02T19:20:33Z

I noticed extract_datetime_component is not included in this header file. Is this file autogenerated? If not, we may need to add it there.

cpp/include/cudf/datetime.hpp

Co-authored-by: Matthew Murray <[email protected]>

cpp/include/cudf/datetime.hpp

python/pylibcudf/pylibcudf/datetime.pyx

Matt711

Approving the python changes. This begs the question, why did cudf::extract_datetime_component work, but not cudf::datetime::extract_datetime_component? It looks like extract_datetime_component is in the datetime namespace.

cpp/include/cudf/datetime.hpp

brandon-b-miller · 2024-10-04T14:31:49Z

/merge

bdice

A few requests / questions.

cpp/include/cudf/datetime.hpp

cpp/src/datetime/datetime_ops.cu

python/pylibcudf/pylibcudf/datetime.pyx

python/pylibcudf/pylibcudf/libcudf/datetime.pxd

brandon-b-miller · 2024-10-07T20:42:56Z

@bdice any last thoughts here?

bdice

My last round of comments was addressed -- approving.

brandon-b-miller added 3 commits September 6, 2024 12:09

impl/tests

fc82848

cleanup

d9eef45

plumb to cudf cython

637f632

brandon-b-miller requested review from a team as code owners September 9, 2024 19:57

brandon-b-miller requested review from wence-, bdice and shrshi September 9, 2024 19:57

github-actions bot added libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. CMake CMake build issue labels Sep 9, 2024

bdice reviewed Sep 9, 2024

View reviewed changes

wence- requested changes Sep 10, 2024

View reviewed changes

brandon-b-miller and others added 4 commits September 16, 2024 08:45

Apply suggestions from code review

80a62ba

Co-authored-by: Bradley Dice <[email protected]> Co-authored-by: Lawrence Mitchell <[email protected]>

merge/resolve/fix

a2dec9e

refactor

da8f3b7

address reviews

be9073e

github-actions bot added the pylibcudf Issues specific to the pylibcudf package label Sep 25, 2024

wence- reviewed Sep 25, 2024

View reviewed changes

cpp/src/datetime/datetime_ops.cu Outdated Show resolved Hide resolved

use macro

1745cb6

brandon-b-miller added feature request New feature or request non-breaking Non-breaking change labels Sep 25, 2024

Merge branch 'branch-24.10' into fea-datetime-component

3457e2c

wence- requested changes Sep 26, 2024

View reviewed changes

cpp/src/datetime/datetime_ops.cu Outdated Show resolved Hide resolved

cpp/src/datetime/datetime_ops.cu Outdated Show resolved Hide resolved

cpp/src/datetime/datetime_ops.cu Outdated Show resolved Hide resolved

address reviews

a6a909c

brandon-b-miller requested a review from wence- September 27, 2024 18:11

Merge branch 'branch-24.12' into fea-datetime-component

616c5dd

brandon-b-miller changed the base branch from branch-24.10 to branch-24.12 October 1, 2024 23:42

use enum in polars code

5fd9468

github-actions bot added the cudf.polars Issues specific to cudf.polars label Oct 2, 2024

Matt711 reviewed Oct 2, 2024

View reviewed changes

cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved

brandon-b-miller and others added 2 commits October 2, 2024 13:02

minor fixes

844e3dc

Update cpp/include/cudf/datetime.hpp

722c6f9

Co-authored-by: Matthew Murray <[email protected]>

Matt711 reviewed Oct 3, 2024

View reviewed changes

cpp/include/cudf/datetime.hpp Show resolved Hide resolved

brandon-b-miller commented Oct 3, 2024

View reviewed changes

python/pylibcudf/pylibcudf/datetime.pyx Outdated Show resolved Hide resolved

brandon-b-miller and others added 2 commits October 3, 2024 12:24

Update python/pylibcudf/pylibcudf/datetime.pyx

af1397c

Merge branch 'branch-24.12' into fea-datetime-component

6ab40ba

Matt711 approved these changes Oct 4, 2024

View reviewed changes

wence- approved these changes Oct 4, 2024

View reviewed changes

cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved

Remove unnecessary whitespace change

fa191ef

bdice requested changes Oct 4, 2024

View reviewed changes

cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved

cpp/src/datetime/datetime_ops.cu Show resolved Hide resolved

python/pylibcudf/pylibcudf/datetime.pyx Show resolved Hide resolved

python/pylibcudf/pylibcudf/libcudf/datetime.pxd Outdated Show resolved Hide resolved

remove INVALID

09626d2

brandon-b-miller requested a review from bdice October 4, 2024 17:14

bdice approved these changes Oct 7, 2024

View reviewed changes

rapids-bot bot merged commit 2d02bdc into rapidsai:branch-24.12 Oct 7, 2024
102 checks passed

Matt711 mentioned this pull request Oct 17, 2024

[FEA] Implement all libcudf modules required by cuDF Python in pylibcudf #15162

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `extract_datetime_component` in `libcudf`/`pylibcudf` #16776

Implement `extract_datetime_component` in `libcudf`/`pylibcudf` #16776

brandon-b-miller commented Sep 9, 2024

bdice Sep 9, 2024

brandon-b-miller Sep 25, 2024

wence- Sep 10, 2024

wence- Sep 10, 2024

wence- left a comment

brandon-b-miller commented Oct 2, 2024

bdice commented Oct 2, 2024

Matt711 commented Oct 2, 2024 •

edited

Loading

Matt711 commented Oct 2, 2024

Matt711 left a comment

brandon-b-miller commented Oct 4, 2024

bdice left a comment

brandon-b-miller commented Oct 7, 2024

bdice left a comment

Implement extract_datetime_component in libcudf/pylibcudf #16776

Implement extract_datetime_component in libcudf/pylibcudf #16776

Conversation

brandon-b-miller commented Sep 9, 2024

bdice Sep 9, 2024

Choose a reason for hiding this comment

brandon-b-miller Sep 25, 2024

Choose a reason for hiding this comment

wence- Sep 10, 2024

Choose a reason for hiding this comment

wence- Sep 10, 2024

Choose a reason for hiding this comment

wence- left a comment

Choose a reason for hiding this comment

brandon-b-miller commented Oct 2, 2024

bdice commented Oct 2, 2024

Matt711 commented Oct 2, 2024 • edited Loading

Matt711 commented Oct 2, 2024

Matt711 left a comment

Choose a reason for hiding this comment

brandon-b-miller commented Oct 4, 2024

bdice left a comment

Choose a reason for hiding this comment

brandon-b-miller commented Oct 7, 2024

bdice left a comment

Choose a reason for hiding this comment

Implement `extract_datetime_component` in `libcudf`/`pylibcudf` #16776

Implement `extract_datetime_component` in `libcudf`/`pylibcudf` #16776

Matt711 commented Oct 2, 2024 •

edited

Loading