Fix merge branch-25.06 into branch-25.08 - #18622
Merged
Merged
Conversation
A 3.1 release candidate changes the way C++ code is emitted for Cython declarations like:
ctypedef some_type alias_name
cdef class Foo:
cdef alias_name whatever
Previously, we would get
new((void*)&(p->whatever)) alias_name();
Now, instead, we get
new((void*)&(p->whatever)) some_type();
This breaks the usage of a `ctypedef` with `std::variant` in the ORC metadata, since we end up with
new((void*)&(p->whatever)) std::variant();
Rather than
new((void*)&(p->whatever)) cudf::io::statistics_type();
The latter is well formed, it default-constructs a `std::variant` that is defined as `std::variant<std::monostate, ...>` in libcudf headers. The former is ill-formed in two ways:
1. A variant must contain at least one default-constructible type
2. Even if we could default-construct, we're now constructing the wrong type.
As a work-around, let us pin to Cython 3.0, until we can figure out the correct way to deal with this and in case there are other fall-outs.
Authors:
- Lawrence Mitchell (https://github.com/wence-)
Approvers:
- Bradley Dice (https://github.com/bdice)
- James Lamb (https://github.com/jameslamb)
URL: NVIDIA#18617
…ze_of (NVIDIA#18607) The `cudf::cast` API requires a double-dispatch to resolve the to and from target types. The internal logic uses `cudf::size_of` on the target type to build the output column. This utility is yet another type-dispatch and is not required since the target-type is already resolved on the 2nd dispatch. Authors: - David Wendt (https://github.com/davidwendt) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) - Devavret Makkar (https://github.com/devavret) URL: NVIDIA#18607
github-actions
Bot
added
libcudf
davidwendt
added
improvement
PointKernel
approved these changes
May 1, 2025
jameslamb
approved these changes
May 1, 2025
davidwendt
changed the title
bdice
approved these changes
May 1, 2025
Contributor
|
We need a non-squash merge on this. I will ask ops to admin non-squash merge this before EOD to avoid blocking some work I'm doing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes merge conflict for auto merger #18620