Remove unneeded pylibcudf.libcudf.wrappers.duration usage in cudf

[mroeschke]

Description

Contributes to #15162

I just assumed since the associated libcudf files just publicly expose C types, we just want to match the name spacing when importing from pylibcudf (avoid importing from pylibcudf.libcudf) and not necessary expose a Python equivalent?

Let me know if I am misunderstanding how to expose these types.

#17010 (comment)

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[Matt711]

I think you got this right. I went through and exposed the other types to see what it would look like, and I don't think you get anything extra from doing that (except following libcudf's API more closely). I would like to hear someone else's thoughts though.

[Matt711]

I think this would become

Suggested change

	ctypedef int32_t duration_D
	ctypedef duration[int32_t, days.period] duration_D

But you'd need to extern duration and days

cdef extern from "cudf/wrappers/durations.hpp" namespace "cudf" nogil:
    cdef cppclass duration[int_type, period]:
        pass
    cdef cppclass days:
        ctypedef int period
    ...

[Matt711]

This would look like after you cimport from durations.pxd

Suggested change

	from libc.stdint cimport int32_t, int64_t
	from libc.stdint cimport int32_t, int64_t
	cdef extern from "cudf/wrappers/timestamps.hpp" namespace "cudf::detail" nogil:
	cdef cppclass time_point[duration]:
	pass
	cdef cppclass timestamp[duration]:
	pass

[Matt711]

And this would become

Suggested change

	ctypedef int32_t timestamp_D
	ctypedef time_point[duration_D] timestamp_D

[vyasr]

The code here seems fine. I don't yet understand what question you're asking. I agree that we don't need to expose a python equivalent for these types. I expect that the only time we will need these is in pylibcudf Cython code for things like Scalar factories. I'm not sure we even need to add these types to pylibcudf at all, outside of pylibcudf.libcudf. Will we ever use them in any place that isn't pylibcudf internals working directly with libcudf types and functions?

[mroeschke]

I don't yet understand what question you're asking. I agree that we don't need to expose a python equivalent for these types.

Yeah the second sentence here answered my question. I was essentially asking if a user should be able to do e.g. from pylibcudf.wrappers.timestamps import timestamp_ns in Python code to get an "equivalent" type.

Will we ever use them in any place that isn't pylibcudf internals working directly with libcudf types and functions?

It appears that we don't even use these types in the pylibcudf internals, and its usage in cudf Python is dead code. I could go ahead and remove these bindings all together since you suggested not even exposing them at all. nevermind there pylibcudf usage of these types in expressions.pyx

[mroeschke]

To not continually edit my prior comment. Based on @vyasr's comment

I'm not sure we even need to add these types to pylibcudf at all, outside of pylibcudf.libcudf.

And the discovery that cudf Python don't need these types either, I won't expose these types under a wrappers namespace. This PR now just cleanups its unneeded usage in cudf

[vyasr]

FWIW I think that we will need these types eventually. Currently we have no way of directly constructing pylibcudf Scalars other than via pyarrow. We definitely want that eventually, and when we address #17054 we will need the types in this PR available to pylibcudf.

[mroeschke]

/merge