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

Promote datasets from experimental to stable #3680

Closed
wants to merge 56 commits into from

Conversation

nv-rliu
Copy link
Contributor

@nv-rliu nv-rliu commented Jun 29, 2023

This PR moves the datasets API from experimental to stable. Users can now do:

from cugraph.datasets import karate

G = karate.get_graph()

The promoted_experimental_warning_wrapper() has been added to the existing experimental.datasets package.

closes #3675

@nv-rliu nv-rliu added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 29, 2023
@nv-rliu nv-rliu added this to the 23.08 milestone Jun 29, 2023
@nv-rliu nv-rliu marked this pull request as ready for review June 29, 2023 20:05
@nv-rliu nv-rliu requested a review from a team as a code owner June 29, 2023 20:05
Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Here's a partial review, I'll add more feedback later.

Some other general feedback is to add unit tests. It would be nice if the meta-data was tested/verified too (eg. the resulting graphs have the same number of vertices and edges as reported in the yaml files) as well as a test to ensure the new warning exceptions are being raised.

python/cugraph/cugraph/datasets/__init__.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/datasets/__init__.py Outdated Show resolved Hide resolved
@nv-rliu nv-rliu marked this pull request as draft June 30, 2023 19:12
@nv-rliu
Copy link
Contributor Author

nv-rliu commented Jul 3, 2023

Update on unit test coverage suggestions by @rlratzel

  • test_dataset.py::test_node_and_edge_count creates a Graph using cugraph.datasets and checks that the number of nodes and edges in the resulting object is the same as the metadata files.
  • test_dataset.py::test_is_directed creates a Graph and checks if was correctly constructed with directed edges by comparing with the metadata file.

Ralph Liu and others added 2 commits July 5, 2023 07:32
Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

Looking better, thanks! We should also figure out how to refactor the tests that still need the various karate derivatives, or do something else so we can stop importing from experimental.

python/cugraph/cugraph/testing/__init__.py Outdated Show resolved Hide resolved
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@acostadon acostadon left a comment

Choose a reason for hiding this comment

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

Looks great !
Not required, but a short README on how to add a new dataset might help users.

@nv-rliu
Copy link
Contributor Author

nv-rliu commented Jul 25, 2023

The changes in this PR branch have been merged into #3712

@nv-rliu nv-rliu closed this Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] Move datasets API from experimental to stable
4 participants