Skip to content

Commit

Permalink
chore(ci): Integrate Python into more CI workflows (#359)
Browse files Browse the repository at this point in the history
This PR adds Python to the coverage and verify workflows (except on
centos7, where the image isn't set up for a supported version of Python
yet). There were also some housekeeping issues identified by various
logs that I noticed in the process (e.g., fixing the Python dev version
format, ensuring that CMAKE_BIN is respected in bootstrap.py).
  • Loading branch information
paleolimbot authored Jan 17, 2024
1 parent 28c5b7b commit b3c952a
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 36 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/coverage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,6 @@ jobs:
path: _coverage

- name: Upload coverage to codecov
uses: codecov/codecov-action@v2
uses: codecov/codecov-action@v3
with:
files: '_coverage/coverage.info,_coverage/r_coverage.json'
files: '_coverage/coverage.info,_coverage/r_coverage.json,_coverage/python_coverage.xml'
25 changes: 2 additions & 23 deletions .github/workflows/python.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
python-version: ['3.10']
python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']

steps:
- uses: actions/checkout@v3
Expand All @@ -59,27 +59,6 @@ jobs:
pytest python/tests -v -s
- name: Run doctests
if: success() && matrix.python-version == '3.10'
if: success() && matrix.python-version == '3.12'
run: |
pytest --pyargs nanoarrow --doctest-modules
- name: Coverage
if: success() && matrix.python-version == '3.10'
run: |
pip uninstall --yes nanoarrow
pip install pytest-cov Cython
pushd python
# Build with Cython + gcc coverage options
pip install -e .
NANOARROW_PYTHON_COVERAGE=1 python setup.py build_ext --inplace
# Run tests + coverage.py (generates .coverage + coverage.xml files)
python -m pytest --cov ./src/nanoarrow
python -m coverage xml
- name: Upload coverage to codecov
if: success() && matrix.python-version == '3.10'
uses: codecov/codecov-action@v2
with:
files: 'python/coverage.xml'
4 changes: 3 additions & 1 deletion .github/workflows/verify.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ jobs:
- {
platform: "centos7",
arch: "amd64",
compose_args: "-e NANOARROW_ACCEPT_IMPORT_GPG_KEYS_ERROR=1"
# Currently the Python on the centos7 image is 3.6, which does not support
# new enough setuptools to build the Python package.
compose_args: "-e NANOARROW_ACCEPT_IMPORT_GPG_KEYS_ERROR=1 -e TEST_PYTHON=0"
}
- {
platform: "ubuntu",
Expand Down
38 changes: 38 additions & 0 deletions ci/scripts/coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,15 @@ case $# in
;;
esac

maybe_activate_venv() {
if [ ! -z "${NANOARROW_PYTHON_VENV}" ]; then
source "${NANOARROW_PYTHON_VENV}/bin/activate"
fi
}

function main() {
maybe_activate_venv

SANDBOX_DIR="${TARGET_NANOARROW_DIR}/_coverage"
if [ -d "${SANDBOX_DIR}" ]; then
rm -rf "${SANDBOX_DIR}"
Expand Down Expand Up @@ -107,6 +115,7 @@ function main() {
--exclude "*/gtest/*" \
--exclude "*/flatcc/*" \
--exclude "*_generated.h" \
--exclude "*nanoarrow/_deps/*" \
--output-file coverage.info

# Generate the html coverage while we're here
Expand Down Expand Up @@ -137,6 +146,35 @@ function main() {
show_header "R package coverage summary"
Rscript -e 'library(covr); print(readRDS("r_coverage.rds"))'
popd

# Build + test Python package with cython/gcc coverage options
show_header "Build + test Python package"
pushd "${SANDBOX_DIR}"
TARGET_NANOARROW_PYTHON_DIR="${TARGET_NANOARROW_DIR}/python"

pushd "${TARGET_NANOARROW_PYTHON_DIR}"
NANOARROW_PYTHON_COVERAGE=1 python -m pip install -e .

# Run tests + coverage.py (generates .coverage with absolute file paths)
python -m pytest --cov ./src/nanoarrow

# Generate HTML report (file paths not important since it's just for viewing)
python -m coverage html
mv htmlcov "${SANDBOX_DIR}/python_htmlcov"

# Move .coverage to the root directory and generate coverage.xml
# (generates relative file paths from the root of the repo)
mv .coverage ..
cp .coveragerc ..
pushd ..
python -m coverage xml
mv coverage.xml "${SANDBOX_DIR}/python_coverage.xml"
mv .coverage "${SANDBOX_DIR}/python_coverage.db"
rm .coveragerc
popd

popd
popd
}

main
58 changes: 58 additions & 0 deletions dev/release/verify-release-candidate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,59 @@ test_r() {
popd
}

activate_or_create_venv() {
if [ ! -z "${NANOARROW_PYTHON_VENV}" ]; then
show_info "Activating virtual environment at ${NANOARROW_PYTHON_VENV}"
# bash on Windows needs venv/Scripts/activate instead of venv/bin/activate
source "${NANOARROW_PYTHON_VENV}/bin/activate" || source "${NANOARROW_PYTHON_VENV}/Scripts/activate"
else
# Try python3 first, then try regular python (e.g., Windows)
if [ -z "${PYTHON_BIN}" ] && python3 --version >/dev/null; then
PYTHON_BIN=python3
elif [ -z "${PYTHON_BIN}" ]; then
PYTHON_BIN=python
fi

show_info "Creating temporary virtual environment using ${PYTHON_BIN}..."
"${PYTHON_BIN}" -m venv "${NANOARROW_TMPDIR}/venv"
# bash on Windows needs venv/Scripts/activate instead of venv/bin/activate
source "${NANOARROW_TMPDIR}/venv/bin/activate" || source "${NANOARROW_TMPDIR}/venv/Scripts/activate"
python -m pip install --upgrade pip
fi
}

test_python() {
show_header "Build and test Python package"
activate_or_create_venv

show_info "Installing build utilities"
python -m pip install --upgrade build

pushd "${NANOARROW_SOURCE_DIR}/python"

show_info "Building Python package"
rm -rf "${NANOARROW_TMPDIR}/python"
python -m build --wheel --outdir "${NANOARROW_TMPDIR}/python"
PYTHON_WHEEL_NAME=$(ls "${NANOARROW_TMPDIR}/python" | grep -e ".whl")

# On Windows bash, pip install needs a Windows-style path
if uname | grep -e "_NT-" >/dev/null; then
pushd "${NANOARROW_TMPDIR}"
PYTHON_WHEEL_PATH="$(pwd -W)/python/${PYTHON_WHEEL_NAME}"
popd
else
PYTHON_WHEEL_PATH="${NANOARROW_TMPDIR}/python/${PYTHON_WHEEL_NAME}"
fi

show_info "Installing Python package"
python -m pip install --force-reinstall "${PYTHON_WHEEL_PATH}[verify]"

show_info "Testing wheel"
python -m pytest -vv

popd
}

ensure_source_directory() {
show_header "Ensuring source directory"

Expand Down Expand Up @@ -346,6 +399,10 @@ test_source_distribution() {
test_r
fi

if [ ${TEST_PYTHON} -gt 0 ]; then
test_python
fi

popd
}

Expand All @@ -359,6 +416,7 @@ test_source_distribution() {
: ${TEST_C:=${TEST_SOURCE}}
: ${TEST_C_BUNDLED:=${TEST_C}}
: ${TEST_R:=${TEST_SOURCE}}
: ${TEST_PYTHON:=${TEST_SOURCE}}

TEST_SUCCESS=no

Expand Down
9 changes: 6 additions & 3 deletions python/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ def copy_or_generate_nanoarrow_c():
is_in_nanoarrow_repo = "nanoarrow.h" in os.listdir(
os.path.join(source_dir, "src", "nanoarrow")
)
has_cmake = os.system("cmake --version") == 0
cmake_bin = os.getenv("CMAKE_BIN")
if not cmake_bin:
cmake_bin = "cmake"
has_cmake = os.system(f"{cmake_bin} --version") == 0

with tempfile.TemporaryDirectory() as build_dir:
if is_in_nanoarrow_repo:
Expand All @@ -206,7 +209,7 @@ def copy_or_generate_nanoarrow_c():
try:
subprocess.run(
[
"cmake",
cmake_bin,
"-B",
build_dir,
"-S",
Expand All @@ -217,7 +220,7 @@ def copy_or_generate_nanoarrow_c():
)
subprocess.run(
[
"cmake",
cmake_bin,
"--install",
build_dir,
"--prefix",
Expand Down
8 changes: 6 additions & 2 deletions python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@ dynamic = ["version"]
readme = "README.md"
description = "Python bindings to the nanoarrow C library"
authors = [{name = "Apache Arrow Developers", email = "[email protected]"}]
maintainers = [{name = "Apache Arrow Developers", email = "[email protected]"}]
license = {text = "Apache-2.0"}
requires-python = ">=3.8"

[project.optional-dependencies]
test = ["pyarrow", "pytest", "numpy"]
verify = ["pytest", "numpy"]

[project.urls]
homepage = "https://arrow.apache.org"
repository = "https://github.com/apache/arrow-nanoarrow"
Homepage = "https://arrow.apache.org"
Repository = "https://github.com/apache/arrow-nanoarrow"
Issues = "https://github.com/apache/arrow-nanoarrow/issues"
Changelog = "https://github.com/apache/arrow-nanoarrow/blob/main/CHANGELOG.md"

[build-system]
requires = [
Expand Down
3 changes: 1 addition & 2 deletions python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def get_version(pkg_path):


# Set some extra flags for compiling with coverage support
if os.getenv("NANOARROW_COVERAGE") == "1":
if os.getenv("NANOARROW_PYTHON_COVERAGE") == "1":
extra_compile_args = ["--coverage"]
extra_link_args = ["--coverage"]
extra_define_macros = [("CYTHON_TRACE", 1)]
Expand All @@ -65,7 +65,6 @@ def get_version(pkg_path):
extra_link_args = []
extra_define_macros = []


setup(
ext_modules=[
Extension(
Expand Down
2 changes: 1 addition & 1 deletion python/src/nanoarrow/_lib.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ cdef class CBufferView:
if format_const != NULL:
snprintf(self._format, sizeof(self._format), "%s", format_const)
else:
snprintf(self._format, sizeof(self._format), "%ds", self._element_size_bits // 8)
snprintf(self._format, sizeof(self._format), "%ds", <int>(self._element_size_bits // 8))

def __getbuffer__(self, Py_buffer *buffer, int flags):
if self._device.device_type != ARROW_DEVICE_CPU:
Expand Down
2 changes: 1 addition & 1 deletion python/src/nanoarrow/_static_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# This file is part of 'miniver': https://github.com/jbweston/miniver

# Replaced by version-bumping scripts at release time
version = "0.4.0dev0"
version = "0.4.0.dev0"

# These values are only set if the distribution was created with 'git archive'
refnames = "$Format:%D$"
Expand Down
2 changes: 1 addition & 1 deletion python/tests/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@


def test_version():
re_py_version = re.compile(r"^[0-9]+\.[0-9]+\.[0-9]+(dev[0-9+])?$")
re_py_version = re.compile(r"^[0-9]+\.[0-9]+\.[0-9]+(\.dev[0-9+])?$")
assert re_py_version.match(na.__version__) is not None


Expand Down

0 comments on commit b3c952a

Please sign in to comment.