Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpp/include/cudf/unary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ std::unique_ptr<cudf::column> is_valid(
* @param mr Device memory resource used to allocate the returned column's device memory
*
* @returns Column of same size as `input` containing result of the cast operation
* @throw cudf::logic_error if `out_type` is not a fixed-width type
* @throw cudf::logic_error if `input` or `out_type` is not a fixed-width type
*/
std::unique_ptr<column> cast(
column_view const& input,
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/unary/cast_ops.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -118,7 +118,7 @@ struct fixed_point_unary_cast {
template <typename From, typename To>
constexpr inline auto is_supported_non_fixed_point_cast()
{
return cudf::is_fixed_width<To>() &&
return cudf::is_fixed_width<From>() && cudf::is_fixed_width<To>() &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

Your PR description says that you're matching the "cudf cast documentation", but the cast will, in fact, fail for those non-fixed-width types, so you're in fact matching the cast implementation. Might be worth a tweak to the PR description…

[Optional] Might also be worth it to update the is_supported_cast function doc in unary.hpp (lines 129-136)…

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@igorpeshansky - agreed about expanding the test above and took your suggestion there. Changed the PR description too. Thanks!
Not sure I see a change to the header docs for is_supported_cast as necessary here though. I feel like the current description of "Check if a cast between two datatypes is supported." is a good one - nice and concise, and I don't know that it needs to spell out all the possible cast failure paths.
The documentation of cast in unary.hpp was a little funny though - it mentioned failing on variable width output_type, but not variable-width inputs. So I updated that one.

// Disallow fixed_point here (requires different specialization)
!(cudf::is_fixed_point<From>() || cudf::is_fixed_point<To>()) &&
// Disallow conversions between timestamps and numeric
Expand Down
11 changes: 10 additions & 1 deletion cpp/tests/unary/cast_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -201,6 +201,15 @@ inline auto make_data_type()
return cudf::data_type{cudf::type_to_id<T>()};
}

TEST(IsSupportedCast, UnsupportedTypes)
{
auto const to_int32 = cudf::data_type{cudf::type_id::INT32};
EXPECT_FALSE(cudf::is_supported_cast(cudf::data_type{cudf::type_id::STRING}, to_int32));
EXPECT_FALSE(cudf::is_supported_cast(cudf::data_type{cudf::type_id::LIST}, to_int32));
EXPECT_FALSE(cudf::is_supported_cast(cudf::data_type{cudf::type_id::STRUCT}, to_int32));
EXPECT_FALSE(cudf::is_supported_cast(cudf::data_type{cudf::type_id::DICTIONARY32}, to_int32));
}

struct CastTimestampsSimple : public cudf::test::BaseFixture {};

TEST_F(CastTimestampsSimple, IsIdempotent)
Expand Down
Loading