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.

Sign up for GitHub

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

Jump to bottom

[FEA] Change the default for Graph instances returned by PropertyGraph.extract_subgraph() to directed #2459

Closed
rlratzel opened this issue Jul 28, 2022 · 0 comments · Fixed by #2460
Closed

[FEA] Change the default for Graph instances returned by PropertyGraph.extract_subgraph() to directed #2459

rlratzel opened this issue Jul 28, 2022 · 0 comments · Fixed by #2460
Assignees
@eriknw
Labels
? - Needs Triage Need team to review and classify breaking Breaking change improvement Improvement / enhancement to an existing function
Milestone
22.08

Comments

@rlratzel
Copy link
Contributor

rlratzel commented Jul 28, 2022 •
edited
Loading

Most known use cases for PropertyGraph.extract_subgraph() require a directed graph (eg. GNN sampling pipelines). The current default is undirected, and users are required to pass a graph instance to create_using to get a directed graph.

Perhaps since the Property Graph itself is directed by nature (ie. the edge data is not being symmetrized), and the majority of users (as far as we know) need directed, the default graph instance returned by extract_subgraph if create_using isn't specified should be directed.

NOTE: I've added the breaking label to this issue since changing a default is technically a breaking change.

See also #2457
@rlratzel rlratzel added ? - Needs Triage Need team to review and classify improvement Improvement / enhancement to an existing function breaking Breaking change labels Jul 28, 2022
@rlratzel rlratzel added this to the 22.08 milestone Jul 28, 2022
@VibhuJawa VibhuJawa mentioned this issue Jul 28, 2022
Closed
@rlratzel rlratzel changed the title [FEA] Change the default for Graph isntances returned by PropertyGraph.extract_subgraph() to directed [FEA] Change the default for Graph instances returned by PropertyGraph.extract_subgraph() to directed Jul 28, 2022
eriknw added a commit to eriknw/cugraph that referenced this issue Jul 28, 2022
@eriknw
Change default return type `PropertyGraph.extract_subgraph() -> cugra…
29f7b9c 
…ph.Graph(directed=True)`

Fixes rapidsai#2459
@eriknw eriknw mentioned this issue Jul 28, 2022
Merged
rapids-bot bot pushed a commit that referenced this issue Jul 28, 2022
@eriknw
Change default return type `PropertyGraph.extract_subgraph() -> cugra…
aca180a 
…ph.Graph(directed=True)` (#2460)

Fixes #2459

The tests already specify the use of directed graphs most places, so this required minimal changes.

Authors:
  - Erik Welch (https://github.com/eriknw)

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

URL: #2460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eriknw eriknw

Labels
? - Needs Triage Need team to review and classify breaking Breaking change improvement Improvement / enhancement to an existing function
Projects
None yet
Milestone
22.08
2 participants
@eriknw @rlratzel