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

[Python][Dev] Switch to using pyproject.toml when building and running CI. #9944

Merged
merged 10 commits into from Dec 12, 2023
11 changes: 8 additions & 3 deletions .github/workflows/Main.yml
Expand Up @@ -66,11 +66,16 @@ jobs:
shell: bash
run: make debug

- name: Set DUCKDB_INSTALL_LIB for ADBC tests
shell: bash
run: echo "DUCKDB_INSTALL_LIB=$(find `pwd` -name "libduck*.so" | head -n 1)" >> $GITHUB_ENV

- name: Test DUCKDB_INSTALL_LIB variable
run: echo $DUCKDB_INSTALL_LIB

- name: Test
shell: bash
run: |
echo "DUCKDB_INSTALL_LIB=$(find `pwd` -name "libduck*.so" | head -n 1)" >> $GITHUB_ENV
make unittestci
run: make unittestci


force-storage:
Expand Down
11 changes: 8 additions & 3 deletions .github/workflows/OSX.yml
Expand Up @@ -56,12 +56,17 @@ jobs:
shell: bash
run: GEN=ninja make debug

- name: Set DUCKDB_INSTALL_LIB for ADBC tests
shell: bash
run: echo "DUCKDB_INSTALL_LIB=$(find `pwd` -name "libduck*.dylib" | head -n 1)" >> $GITHUB_ENV

- name: Test DUCKDB_INSTALL_LIB variable
run: echo $DUCKDB_INSTALL_LIB

- name: Test
if: ${{ !startsWith(github.ref, 'refs/tags/v') }}
shell: bash
run: |
echo "DUCKDB_INSTALL_LIB=$(find `pwd` -name "libduck*.dylib" | head -n 1)" >> $GITHUB_ENV
make unittestci
run: make unittestci

- name: Amalgamation
if: ${{ !startsWith(github.ref, 'refs/tags/v') }}
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/Python.yml
Expand Up @@ -82,7 +82,7 @@ jobs:
run: |
export DISTUTILS_C_COMPILER_LAUNCHER=ccache
# TODO: Use ccache inside container, see https://github.com/pypa/cibuildwheel/issues/1030
cibuildwheel --output-dir wheelhouse --config-file cibw.toml duckdb_tarball
cibuildwheel --output-dir wheelhouse --config-file pyproject.toml duckdb_tarball
Copy link
Member

Choose a reason for hiding this comment

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

Bit of a nitpick, but this is the default value for config-file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah alright, if there are other issues or the PR requires touching in another way I'll address it, if not I'll leave it as is 👍


manylinux-extensions-x64:
# Builds extensions for linux_md64_gcc4
Expand Down Expand Up @@ -204,7 +204,7 @@ jobs:
if [[ "$GITHUB_REF" =~ ^refs/tags/v.+$ ]] ; then
export CIBW_TEST_COMMAND='python -m pytest {project}/tests/fast/api/test_dbapi00.py --verbose'
fi
cibuildwheel --output-dir wheelhouse --config-file cibw.toml duckdb_tarball
cibuildwheel --output-dir wheelhouse --config-file pyproject.toml duckdb_tarball
ls wheelhouse

- name: Deploy
Expand Down Expand Up @@ -264,7 +264,7 @@ jobs:
if [[ "$GITHUB_REF" =~ ^refs/tags/v.+$ ]] ; then
export CIBW_TEST_COMMAND='python -m pytest {project}/tests/fast/api/test_dbapi00.py --verbose'
fi
cibuildwheel --output-dir wheelhouse --config-file cibw.toml duckdb_tarball
cibuildwheel --output-dir wheelhouse --config-file pyproject.toml duckdb_tarball

- name: Deploy
env:
Expand Down Expand Up @@ -332,7 +332,7 @@ jobs:
if [[ "$GITHUB_REF" =~ ^refs/tags/v.+$ ]] ; then
export CIBW_TEST_COMMAND='python -m pytest {project}/tests/fast/api/test_dbapi00.py --verbose'
fi
cibuildwheel --output-dir wheelhouse --config-file cibw.toml duckdb_tarball
cibuildwheel --output-dir wheelhouse --config-file pyproject.toml duckdb_tarball

- name: Deploy
env:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/Regression.yml
Expand Up @@ -242,7 +242,7 @@ jobs:
shell: bash
run: |
cd tools/pythonpkg
python setup.py install --user
pip install --use-pep517 . --user
cd ../..

- name: Run New Version
Expand All @@ -262,7 +262,7 @@ jobs:
run: |
git clone --branch ${{ env.BASE_BRANCH }} https://github.com/duckdb/duckdb.git --depth=1
cd duckdb/tools/pythonpkg
python setup.py install --user
pip install --use-pep517 . --user
cd ../../..

- name: Run Current Version
Expand Down
3 changes: 1 addition & 2 deletions tools/pythonpkg/MANIFEST.in
Expand Up @@ -3,5 +3,4 @@ recursive-include duckdb *.h *.hpp *.cpp
include duckdb_python.cpp
recursive-include src *.h *.hpp *.cpp
recursive-include duckdb-stubs *.pyi
recursive-include duckdb
include cibw.toml
recursive-include duckdb
15 changes: 14 additions & 1 deletion tools/pythonpkg/cibw.toml → tools/pythonpkg/pyproject.toml
@@ -1,4 +1,17 @@
# Config for cibuildwheel, some values are overridden by env variables of specific jobs
[build-system]
requires = [
"setuptools>=60",
"setuptools_scm>=6.4",
"pybind11>=2.6.0",
"wheel>=0.38.0"
]
build-backend = "setuptools.build_meta"

[tool.setuptools_scm]
root = "../.."
local_scheme = {env = "SETUPTOOLS_SCM_LOCAL_SCHEME", default = "no-local-version"}

### CI Builwheel configurations ###

# Default config runs all tests and requires at least one extension to be tested against
[tool.cibuildwheel]
Expand Down
5 changes: 3 additions & 2 deletions tools/pythonpkg/setup.py
Expand Up @@ -328,10 +328,13 @@ def setup_data_files(data_files):
'duckdb-stubs',
'duckdb-stubs.functional',
'duckdb-stubs.typing',
'duckdb-stubs.value',
'duckdb-stubs.value.constant',
'adbc_driver_duckdb',
]

spark_packages = [
'duckdb.experimental',
'duckdb.experimental.spark',
'duckdb.experimental.spark.sql',
'duckdb.experimental.spark.errors',
Expand All @@ -352,8 +355,6 @@ def setup_data_files(data_files):
packages=packages,
include_package_data=True,
python_requires='>=3.7.0',
setup_requires=setup_requires + ["setuptools_scm<7.0.0", 'pybind11>=2.6.0'],
use_scm_version=setuptools_scm_conf,
tests_require=['google-cloud-storage', 'mypy', 'pytest'],
classifiers=[
'Topic :: Database :: Database Engines/Servers',
Expand Down
4 changes: 2 additions & 2 deletions tools/pythonpkg/src/pyexpression.cpp
Expand Up @@ -10,7 +10,7 @@ namespace duckdb {

DuckDBPyExpression::DuckDBPyExpression(unique_ptr<ParsedExpression> expr_p, OrderType order_type,
OrderByNullType null_order)
: expression(std::move(expr_p)), order_type(order_type), null_order(null_order) {
: expression(std::move(expr_p)), null_order(null_order), order_type(order_type) {
if (!expression) {
throw InternalException("DuckDBPyExpression created without an expression");
}
Expand Down Expand Up @@ -325,7 +325,7 @@ shared_ptr<DuckDBPyExpression> DuckDBPyExpression::CaseExpression(const DuckDBPy
// Add NULL as default Else expression
auto &internal_expression = reinterpret_cast<duckdb::CaseExpression &>(*case_expr->expression);
internal_expression.else_expr = make_uniq<duckdb::ConstantExpression>(Value(LogicalTypeId::SQLNULL));
return std::move(case_expr);
return case_expr;
}

shared_ptr<DuckDBPyExpression> DuckDBPyExpression::FunctionExpression(const string &function_name,
Expand Down
2 changes: 1 addition & 1 deletion tools/pythonpkg/src/pyresult.cpp
Expand Up @@ -177,7 +177,7 @@ unique_ptr<NumpyResultConversion> DuckDBPyResult::InitializeNumpyConversion(bool

auto conversion =
make_uniq<NumpyResultConversion>(result->types, initial_capacity, result->client_properties, pandas);
return std::move(conversion);
return conversion;
}

py::dict DuckDBPyResult::FetchNumpyInternal(bool stream, idx_t vectors_per_chunk,
Expand Down