-
Notifications
You must be signed in to change notification settings - Fork 326
Add shellcheck to pre-commit and fix warnings #4954
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
Changes from 1 commit
afb0bf3
8ca6c9c
b28a75e
c5faa3b
31675e4
8becb2a
c4f3f8f
2ce9487
21c6e28
614eb0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,8 @@ | |
set -euo pipefail | ||
|
||
if [[ "${RAPIDS_CUDA_VERSION}" == "11.8.0" ]]; then | ||
CONDA_CUDA_VERSION="11.8" | ||
DGL_CHANNEL="dglteam/label/th23_cu118" | ||
else | ||
CONDA_CUDA_VERSION="12.1" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I love when these |
||
DGL_CHANNEL="dglteam/label/th23_cu121" | ||
fi | ||
|
||
|
@@ -18,9 +16,12 @@ PYTHON_CHANNEL=$(rapids-download-conda-from-s3 python) | |
rapids-logger "Create test conda environment" | ||
. /opt/conda/etc/profile.d/conda.sh | ||
|
||
export RAPIDS_VERSION="$(rapids-version)" | ||
export RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)" | ||
export RAPIDS_VERSION_NUMBER="$RAPIDS_VERSION_MAJOR_MINOR" | ||
RAPIDS_VERSION="$(rapids-version)" | ||
export RAPIDS_VERSION | ||
RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)" | ||
export RAPIDS_VERSION_MAJOR_MINOR | ||
RAPIDS_VERSION_NUMBER="$RAPIDS_VERSION_MAJOR_MINOR" | ||
export RAPIDS_VERSION_NUMBER | ||
|
||
rapids-dependency-file-generator \ | ||
--output conda \ | ||
|
@@ -38,12 +39,14 @@ conda activate docs | |
|
||
rapids-print-env | ||
|
||
export RAPIDS_DOCS_DIR="$(mktemp -d)" | ||
RAPIDS_DOCS_DIR="$(mktemp -d)" | ||
export RAPIDS_DOCS_DIR | ||
|
||
rapids-logger "Build CPP docs" | ||
pushd cpp/doxygen | ||
doxygen Doxyfile | ||
export XML_DIR_LIBCUGRAPH="$(pwd)/xml" | ||
XML_DIR_LIBCUGRAPH="$(pwd)/xml" | ||
export XML_DIR_LIBCUGRAPH | ||
mkdir -p "${RAPIDS_DOCS_DIR}/libcugraph/xml_tar" | ||
tar -czf "${RAPIDS_DOCS_DIR}/libcugraph/xml_tar"/xml.tar.gz -C xml . | ||
popd | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,19 +5,19 @@ set -eoxu pipefail | |||||
|
||||||
package_name=$1 | ||||||
|
||||||
python_package_name=$(echo ${package_name}|sed 's/-/_/g') | ||||||
python_package_name=${package_name//-/_} | ||||||
|
||||||
# Run smoke tests for aarch64 pull requests | ||||||
arch=$(uname -m) | ||||||
if [[ "${arch}" == "aarch64" && ${RAPIDS_BUILD_TYPE} == "pull-request" ]]; then | ||||||
python ./ci/wheel_smoke_test_${package_name}.py | ||||||
python ./ci/wheel_smoke_test_"{package_name}".py | ||||||
else | ||||||
# Test runs that include tests that use dask require | ||||||
# --import-mode=append. See test_python.sh for details. | ||||||
# FIXME: Adding PY_IGNORE_IMPORTMISMATCH=1 to workaround conftest.py import | ||||||
# mismatch error seen by nx-cugraph after using pytest 8 and | ||||||
# --import-mode=append. | ||||||
RAPIDS_DATASET_ROOT_DIR=`pwd`/datasets \ | ||||||
RAPIDS_DATASET_ROOT_DIR=$(pwd)/datasets \ | ||||||
PY_IGNORE_IMPORTMISMATCH=1 \ | ||||||
DASK_WORKER_DEVICES="0" \ | ||||||
DASK_DISTRIBUTED__SCHEDULER__WORKER_TTL="1000s" \ | ||||||
|
@@ -28,5 +28,5 @@ else | |||||
--import-mode=append \ | ||||||
--benchmark-disable \ | ||||||
-k "not test_property_graph_mg and not test_bulk_sampler_io" \ | ||||||
./python/${package_name}/${python_package_name}/tests | ||||||
./python/"${package_name}"/"${python_package_name}"/tests | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Since there isn't anything else here like wildcards that need to be evaluated, I have a mild preference for just wrapping the entire thing in double quotes, instead of having 2 inner sets of double quotes. Think that's a little easier to read and to modify. |
||||||
fi |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||||||||||||||
#!/bin/bash | ||||||||||||||||||
# Copyright (c) 2019-2021, NVIDIA CORPORATION. | ||||||||||||||||||
# Copyright (c) 2019-2025, NVIDIA CORPORATION. | ||||||||||||||||||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||
# you may not use this file except in compliance with the License. | ||||||||||||||||||
# You may obtain a copy of the License at | ||||||||||||||||||
|
@@ -48,22 +48,22 @@ get_ipython().run_cell_magic=my_run_cell_magic | |||||||||||||||||
NO_COLORS=--colors=NoColor | ||||||||||||||||||
EXITCODE=0 | ||||||||||||||||||
NBTMPDIR=${WORKSPACE}/tmp | ||||||||||||||||||
mkdir -p ${NBTMPDIR} | ||||||||||||||||||
mkdir -p "${NBTMPDIR}" | ||||||||||||||||||
|
||||||||||||||||||
for nb in $*; do | ||||||||||||||||||
NBFILENAME=$1 | ||||||||||||||||||
for nb in "$@"; do | ||||||||||||||||||
NBFILENAME=$nb | ||||||||||||||||||
NBNAME=${NBFILENAME%.*} | ||||||||||||||||||
NBNAME=${NBNAME##*/} | ||||||||||||||||||
NBTESTSCRIPT=${NBTMPDIR}/${NBNAME}-test.py | ||||||||||||||||||
shift | ||||||||||||||||||
|
||||||||||||||||||
echo -------------------------------------------------------------------------------- | ||||||||||||||||||
echo STARTING: ${NBNAME} | ||||||||||||||||||
echo STARTING: "${NBNAME}" | ||||||||||||||||||
echo -------------------------------------------------------------------------------- | ||||||||||||||||||
jupyter nbconvert --to script ${NBFILENAME} --output ${NBTMPDIR}/${NBNAME}-test | ||||||||||||||||||
echo "${MAGIC_OVERRIDE_CODE}" > ${NBTMPDIR}/tmpfile | ||||||||||||||||||
cat ${NBTESTSCRIPT} >> ${NBTMPDIR}/tmpfile | ||||||||||||||||||
mv ${NBTMPDIR}/tmpfile ${NBTESTSCRIPT} | ||||||||||||||||||
jupyter nbconvert --to script "${NBFILENAME}" --output "${NBTMPDIR}"/"${NBNAME}"-test | ||||||||||||||||||
echo "${MAGIC_OVERRIDE_CODE}" > "${NBTMPDIR}"/tmpfile | ||||||||||||||||||
cat "${NBTESTSCRIPT}" >> "${NBTMPDIR}"/tmpfile | ||||||||||||||||||
mv "${NBTMPDIR}"/tmpfile "${NBTESTSCRIPT}" | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Similar to my comments above. |
||||||||||||||||||
|
||||||||||||||||||
echo "Running \"ipython ${NO_COLORS} ${NBTESTSCRIPT}\" on $(date)" | ||||||||||||||||||
echo | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you expand this a bit? I'd really like to see it cover stuff like https://github.com/rapidsai/cugraph/blob/branch-25.04/datasets/get_test_data.sh and https://github.com/rapidsai/cugraph/blob/branch-25.04/scripts/dask/run-dask-process.sh too
If you want to just remove this exclusion and have it cover all shell scripts in this round, I don't mind that making the PR larger.
That'd also be one less repo to have to modify in the second round of this that covers
build.sh
scripts.