Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to update calls to cuDF Series ctors when calling with a Buffer type #1908

Closed
rlratzel opened this issue Oct 27, 2021 · 3 comments · Fixed by #1901
Closed

Need to update calls to cuDF Series ctors when calling with a Buffer type #1908

rlratzel opened this issue Oct 27, 2021 · 3 comments · Fixed by #1901
Assignees
Labels
? - Needs Triage Need team to review and classify
Milestone

Comments

@rlratzel
Copy link
Contributor

rapidsai/cudf#9370 changed support for howSeries object are to be constructed with Buffer objects.

For instance, this example is no longer supported:

import numpy as np
import cudf

tmp = cudf.Series([1, 2, 3])
print(tmp);

buffer = cudf.core.buffer.Buffer(tmp.data)
tmp2 = cudf.Series(data=buffer, dtype=np.dtype("int32"))
print(tmp2)

The supported pattern is now:

...
buffer = cudf.core.buffer.Buffer(tmp.data)
col = cudf.core.column.build_column(buffer, dtype=np.dtype("int32"))
tmp2 = cudf.Series._from_data({SERIES_NAME: col})

cc @ChuckHastings @vyasr

@rlratzel rlratzel added the ? - Needs Triage Need team to review and classify label Oct 27, 2021
@rlratzel rlratzel added this to the 21.12 milestone Oct 27, 2021
@rlratzel rlratzel self-assigned this Oct 27, 2021
rapids-bot bot pushed a commit that referenced this issue Oct 27, 2021
…r handling non-renumbered Graphs (#1901)

* Updates for latest cuDF which changes how `Series` objects are constructed from `Buffer` types.
* Updated `ci/gpu/build.sh` to add the conda env `bin` dir to `PATH`, since it was not finding the correct `cmake` during a local run.
* Cleaned up various cython imports.
* Added test and bug fix to ensure a graph that has not been renumbered is handled correctly by `cugraph.subgraph()`.
* Updated `pytest.ini` for users that don't have the required extra pytest plugins installed, left comments in place to for users to re-enable.
* Minor test and code cleanup.

closes #1908 
closes #1899 

_Note: this PR was originally intended to be two PRs: one for the `subgraph` bug fix, and another for the `cudf.Series` updates. I accidentally pushed the Series updates to the wrong branch, hence this combined PR. I can separate them if reviewing is too difficult._

Authors:
  - Rick Ratzel (https://github.com/rlratzel)

Approvers:
  - Chuck Hastings (https://github.com/ChuckHastings)
  - Brad Rees (https://github.com/BradReesWork)
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: #1901
@vyasr
Copy link
Contributor

vyasr commented Oct 28, 2021

@rlratzel @ChuckHastings I'm glad this was able to get resolved relatively quickly. As we discussed today I'll plan to be more conservative with future changes, but I think it would also be useful for us to meet at some point to get a sense of what parts of cuDF's API cuGraph might be using. There are many aspects that we would consider internal and may eventually want to hide, but some of them may need to stay public or at least stable for other RAPIDS libraries to use.

@ChuckHastings
Copy link
Collaborator

I'll yield to @rlratzel on most of these issues. He leads the python side of things.

I believe we are going to dramatically refactor/simplify much of our interface over the next few months. It seems like a natural time to review all points where we touch cudf as the python side is refactored. I think much of what is there was copied from python/cython in cudf at the time when we needed to implement something. I have no doubt that there are more examples where we are doing things using techniques that are not preferred.

@vyasr
Copy link
Contributor

vyasr commented Nov 1, 2021

OK sounds good @rlratzel I don't know if we necessarily need to have a dedicated conversation for this, that's up to you (perhaps after you've planned more of your refactoring). I'll keep alerting you (and marking relevant PRs) as we make further internal changes in cuDF that may impact you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
? - Needs Triage Need team to review and classify
Projects
None yet
3 participants