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

adding C/C++ API docs #3938

Merged
merged 46 commits into from
Nov 21, 2023
Merged

adding C/C++ API docs #3938

merged 46 commits into from
Nov 21, 2023

Conversation

BradReesWork
Copy link
Member

@BradReesWork BradReesWork commented Oct 16, 2023

adding C and C++ API
Adding WholeGraph APIs
Adding cugraph-ops APIs

closes #3406

@BradReesWork BradReesWork requested review from a team as code owners October 16, 2023 16:48
@BradReesWork BradReesWork marked this pull request as draft October 16, 2023 16:49
@BradReesWork BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 16, 2023
@BradReesWork BradReesWork added this to the 23.12 milestone Oct 16, 2023
@BradReesWork BradReesWork changed the title adding C/C__ API docs adding C/C++ API docs Oct 16, 2023
Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

LGTM.

@BradReesWork BradReesWork marked this pull request as ready for review October 18, 2023 13:29
@BradReesWork BradReesWork requested a review from a team as a code owner October 18, 2023 13:29
@BradReesWork BradReesWork requested a review from rlratzel October 18, 2023 13:29
@ajschmidt8
Copy link
Member

@BradReesWork, if the Sphinx errors in CI are ambiguous, you should be able to add the -v flag to the line below to get a proper stacktrace:

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.

just saw the verbose flag still there.

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.

Looks good, I just had a few comments and questions.

docs/cugraph/source/api_docs/index.rst Outdated Show resolved Hide resolved
docs/cugraph/source/basics/cugraph_intro.md Outdated Show resolved Hide resolved
docs/cugraph/source/conf.py Outdated Show resolved Hide resolved
build.sh Show resolved Hide resolved
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Nice. Looks good. I left a few comments.

We'll also need to update the docs website here to reflect this new documentation structure for the graph libraries: https://docs.rapids.ai/api#libcugraph.

This can be done by editing the versions map here: https://github.com/rapidsai/docs/blob/ce17ee4e9614ed52cea4a6f10aabd57a53cfb08f/_data/docs.yml#L175-L185

Finally, I recommend reviewing the GHAs docs build job logs. There are quite a few warnings and errors. It might not be realistic to resolve all the warnings in this PR, but we should eventually try to fix them all. Once they're all fixed, we can enable the -W flag for Sphinx, which will treat warnings as errors and help the Graph team catch silly documentation mistakes. We've enabled that flag on other RAPIDS libraries.

build.sh Show resolved Hide resolved
ci/build_docs.sh Outdated Show resolved Hide resolved
ci/build_docs.sh Outdated Show resolved Hide resolved
ci/build_docs.sh Outdated Show resolved Hide resolved
BradReesWork and others added 3 commits November 20, 2023 10:44
Co-authored-by: AJ Schmidt <[email protected]>
Co-authored-by: AJ Schmidt <[email protected]>
Co-authored-by: AJ Schmidt <[email protected]>
@BradReesWork
Copy link
Member Author

@ajschmidt8 - I did notice a (large) number of warring with I was building as well. @acostadon is working on a 24.02 PR that will address all those warnings as well as a number of other items (missing functions, etc.)

Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Suggesting one more set of changes via the diff below (click to expand it).

The changes include:

  • Removing unnecessary conditional statements
  • Using an environment variable for the XML_DIR_LIBCUGRAPH directory
  • Disabling Doxygen HTML generation
  • Adding an entry to update-version.sh for build.sh
Diff

diff --git a/build.sh b/build.sh
index 5eaa7c420..23716fcd2 100755
--- a/build.sh
+++ b/build.sh
@@ -417,10 +417,7 @@ if hasArg docs || hasArg all; then
 
     for PROJECT in libcugraphops libwholegraph; do
         XML_DIR="${REPODIR}/docs/cugraph/${PROJECT}"
-        if [ -d ${XML_DIR} ]; then
-            echo "removing ${XML_DIR} docs dir"
-            rm -r ${XML_DIR}
-        fi
+        rm -rf ${XML_DIR}
         mkdir -p "${XML_DIR}"
         export XML_DIR_${PROJECT^^}="$XML_DIR"
 
@@ -433,15 +430,11 @@ if hasArg docs || hasArg all; then
     cd ${LIBCUGRAPH_BUILD_DIR}
     cmake --build "${LIBCUGRAPH_BUILD_DIR}" -j${PARALLEL_LEVEL} --target docs_cugraph ${VERBOSE_FLAG}
 
-    if [ -d ${REPODIR}/docs/cugraph/libcugraph ]; then
-        echo "removing libcugraph docs dir"
-        rm -r ${REPODIR}/docs/cugraph/libcugraph
-    fi
     echo "making libcugraph doc dir"
+    rm -rf ${REPODIR}/docs/cugraph/libcugraph
     mkdir -p ${REPODIR}/docs/cugraph/libcugraph
 
-    mv ${REPODIR}/cpp/doxygen/xml   ${REPODIR}/docs/cugraph/libcugraph/_xml
-    mv ${REPODIR}/cpp/doxygen/html  ${REPODIR}/docs/cugraph/libcugraph/html
+    export XML_DIR_LIBCUGRAPH="${REPODIR}/cpp/doxygen/xml"
 
     cd ${REPODIR}/docs/cugraph
     make html
diff --git a/ci/build_docs.sh b/ci/build_docs.sh
index 1aef056c2..3f765704b 100755
--- a/ci/build_docs.sh
+++ b/ci/build_docs.sh
@@ -52,6 +52,7 @@ done
 rapids-logger "Build CPP docs"
 pushd cpp/doxygen
 doxygen Doxyfile
+export XML_DIR_LIBCUGRAPH="$(pwd)/xml"
 popd
 
 rapids-logger "Build Python docs"
diff --git a/ci/release/update-version.sh b/ci/release/update-version.sh
index 69eb085e7..414ca8c58 100755
--- a/ci/release/update-version.sh
+++ b/ci/release/update-version.sh
@@ -54,6 +54,9 @@ sed_runner "s/set(cugraph_version .*)/set(cugraph_version ${NEXT_FULL_TAG})/g" p
 sed_runner 's/version = .*/version = '"'${NEXT_SHORT_TAG}'"'/g' docs/cugraph/source/conf.py
 sed_runner 's/release = .*/release = '"'${NEXT_FULL_TAG}'"'/g' docs/cugraph/source/conf.py
 
+# build.sh script
+sed_runner 's/RAPIDS_VERSION=.*/RAPIDS_VERSION='${NEXT_SHORT_TAG}'/g' build.sh
+
 # Centralized version file update
 # NOTE: Any script that runs in CI will need to use gha-tool `rapids-generate-version`
 # and echo it to `VERSION` file to get an alpha spec of the current version
diff --git a/cpp/doxygen/Doxyfile b/cpp/doxygen/Doxyfile
index 054f66c4e..6946bd38b 100644
--- a/cpp/doxygen/Doxyfile
+++ b/cpp/doxygen/Doxyfile
@@ -1232,7 +1232,7 @@ IGNORE_PREFIX          =
 # If the GENERATE_HTML tag is set to YES, doxygen will generate HTML output
 # The default value is: YES.
 
-GENERATE_HTML          = YES
+GENERATE_HTML          = NO
 
 # The HTML_OUTPUT tag is used to specify where the HTML docs will be put. If a
 # relative path is entered the value of OUTPUT_DIRECTORY will be put in front of
diff --git a/docs/cugraph/source/conf.py b/docs/cugraph/source/conf.py
index 6c4b1e2ed..3f7ef7deb 100644
--- a/docs/cugraph/source/conf.py
+++ b/docs/cugraph/source/conf.py
@@ -208,7 +208,7 @@ linkcode_resolve = make_linkcode_resolve(
 )
 
 breathe_projects = {
-    'libcugraph': '../libcugraph/_xml',
+    'libcugraph': os.environ['XML_DIR_LIBCUGRAPH'],
     'libcugraphops': os.environ['XML_DIR_LIBCUGRAPHOPS'],
     'libwholegraph': os.environ['XML_DIR_LIBWHOLEGRAPH']
 }

@BradReesWork
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit fceb6b7 into rapidsai:branch-23.12 Nov 21, 2023
70 checks passed
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.

Convert C/C++ API doc to Sphynx
7 participants