fix: Upstream API changes in cuDF 24.10 #3309
Conversation
martindurant
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
There was a problem hiding this comment.
Does this need to be coordinated with a cuDF release? If we merge this, would we gain the ability to work with cuDF >= 24.10 and lose the ability to work with cuDF < 24.10?
Also, I should be careful not to merge this PR without resetting the rapidjson submodule.
src/awkward/operations/ak_to_cudf.py
Outdated
| @@ -18,4 +18,4 @@ def to_cudf( | |||
| """ | |||
| import cudf | |||
|
|
|||
| return cudf.Series(array.layout._to_cudf(cudf, None, len(array))) | |||
| return cudf.Series._from_column(array.layout._to_cudf(cudf, None, len(array))) | |||
There was a problem hiding this comment.
This functionality has become private. Is it something we're not supposed to do anymore?
There was a problem hiding this comment.
It would be worth finding the git blame on this, and asking on their repo
There was a problem hiding this comment.
Here is the PR rapidsai/cudf#16454
|
I should fix this to check the existence of the class method, so that it works for both new and old versions. I am using this branch for the moment for making my akimbo talk :) |
did I make it go away? |
Sorry, it's still there in the current diff:
|
|
How do I undo it? :| |
|
For next time, just delete your fork (locally and on GitHub) and make a new one. (That's how I would do it, just to avoid getting into the weeds of how git submodules work.) It is very unlikely that we'll be changing it again. For this time, I can recreate the PR, making the same changes as in this PR. |
|
OK, thank you. Perhaps I need to go on a submodule course, if such a thing exists. |
|
I don't know of a better way to do header-only C++ dependencies (with random C++ libraries, not necessarily ones that have a CMake convenience). But other than that, I would rather not use them. They don't work the same way that the rest of git does. Here are diffs, side by side: if you don't see any differences apart from the rapidjson directory (I don't), then I'll merge it as soon as the tests pass. This PR: https://github.com/scikit-hep/awkward/pull/3309/files |
This is an exact copy of PR #3309, which was closed in favor of this one (to avoid issues with the `rapidjson` submodule).
No description provided.