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

Update graph_classes.py #1914

Closed
wants to merge 1 commit into from
Closed

Conversation

bmg-pcl
Copy link

@bmg-pcl bmg-pcl commented Oct 28, 2021

Clarify exception message on Graph constructor.

Colleague lost some time tracing this!

Clarify exception message on Graph constructor. 

Colleague lost some time tracing this!
@bmg-pcl bmg-pcl requested a review from a team as a code owner October 28, 2021 22:59
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

1 similar comment
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

@rlratzel rlratzel added bug Something isn't working non-breaking Non-breaking change labels Oct 28, 2021
@rlratzel rlratzel added this to the 21.12 milestone Oct 28, 2021
@rlratzel
Copy link
Contributor

Thank you for submitting this and we're very sorry for the lost time.

@bmg-pcl
Copy link
Author

bmg-pcl commented Oct 29, 2021

Thank you for the quick merge! We're migrating some graph processing codebases to cuGraph and enjoying so far.

I'll do a sweep through exception messages and propose any other related clarifications in a separate PR.

@@ -70,7 +70,7 @@ def __init__(self, m_graph=None, directed=False):
edge_attr=weights)
else:
msg = (
"Graph can only be initialized using MultiGraph "
"Graph can only be initialized using cuGraph or networkX MultiGraph object."
Copy link
Contributor

Choose a reason for hiding this comment

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

NetworkX MultiGraph graphs are not yet supported.

>>> import pandas as pd
>>> import networkx as nx
>>>
>>> import cudf
>>> import cugraph
>>>
>>> df = pd.DataFrame({"src":[0,1,2],"dst":[1,2,0]})
>>>
>>> nxMG = nx.from_pandas_edgelist(df, create_using=nx.MultiGraph,
...                                source="src", target="dst")
>>>
>>> cgMG = cugraph.MultiGraph()
>>> cgMG.from_pandas_edgelist(df, source="src", destination="dst")
>>>
>>> G = cugraph.Graph(m_graph=nxMG)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/conda/envs/rapids/lib/python3.8/site-packages/cugraph-21.12.0a0+57.gcb65876a-py3.8-linux-x86_64.egg/cugraph/structure/graph_classes.py", line 62, in __init__
    elist = m_graph.view_edge_list()
AttributeError: 'MultiGraph' object has no attribute 'view_edge_list'
>>>
>>> G = cugraph.Graph(m_graph=cgMG)
>>>
Suggested change
"Graph can only be initialized using cuGraph or networkX MultiGraph object."
"Graph can only be initialized using a cugraph.MultiGraph object."

@rlratzel
Copy link
Contributor

ok to test

@rlratzel
Copy link
Contributor

@bmg-pcl - it looks like your change failed our automated style check:

>>>> FAILED: flake8 style check; begin output
python/cugraph/cugraph/structure/graph_classes.py:73:80: E501 line too long (96 > 79 characters)

so you may have to split the message across two lines.

@rlratzel
Copy link
Contributor

rlratzel commented Nov 3, 2021

I've moved the changes here to #1925 . Thank you @bmg-pcl for your contribution and for bringing this to our attention.

@rlratzel rlratzel closed this Nov 3, 2021
rapids-bot bot pushed a commit that referenced this pull request Nov 5, 2021
…invalid MultiGraph is passed in (#1925)

* Updated error message and using a proper TypeError exception when an invalid MultiGraph is passed in.
* Added test to verify.

This PR replaces #1914 since that one is not passing the style check and the author has not responded.

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

Approvers:
  - Brad Rees (https://github.com/BradReesWork)

URL: #1925
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants