Skip to content

Conversation

evertlammerts
Copy link
Collaborator

@evertlammerts evertlammerts commented Oct 9, 2025

Still in draft, not all builds are passing yet.

Adding 3.14 support triggered some janitorial work:

  • Catching import errors for pyarrow.
  • Moving from the deprecated typing._UnionGenericAlias to check whether a given object is a Union to isinstance(obj, types.UnionType) for >= py310 and typing.get_origin(obj) is typing.Union as fallback. This meant running the importcache code generation scripts, which needed to output a new header (and had a small bug).
  • Moving from the deprecated pandas chained assignment - which will never work with Copy-on-Write enabled in Pandas, which in turn will be the default from pandas 3.0 onwards - to a delete-and-write approach for tz_localize columns in Pandas. Apart from the chained assignment issue, the tz is now also explicitly part of the column's datatype, and pandas doesn't like it when we change the timezone and throws a FutureWarning: Setting an item of incompatible dtype is deprecated and will raise in a future error of pandas. Value '<DatetimeArray>.

I'll also remove py313 from the "fast" sanity check of the packaging workflow.

Since I'm back in workflows and tests again I'm also evaluating some suggestions from #59:

  • Randomized testing seems usefule, but was throwing a few too many errors to put it in as part of this diff. So for now disabled again but will probably add in a follow up. It already surfaced the issue reported in Release 1.4.1 broke Ibis #108, erroring out in tests that referenced duckdb.typing.<TYPE> without explicitly importing duckdb.typing. I moved all references from duckdb.typing in the tests to duckdb.pytypes.
  • The pytest-xdist plugin, with mixed results. There is some speedup in CI, but not very much, and at the cost of readability. It also seemed to hang during completion now and then. Not sure whether / how I'll keep it in.
  • Added --quiet to uv export in packaging_wheels. The output has been very useful to set the env markers correctly for all dependencies (which is much better now than it was in-tree) but I agree it's a bit noisy. It was already set for sdists, so may as well have it here as well.
  • I've put up a small PR to change duckdb's ccache and sccache detection. I'll check whether we can better use hendrikmuhs/ccache-action in our workflows, like duckdb does, and evaluate the performance of sccache as well. Locally it doesn't make much difference for me.

Tishj
Tishj previously approved these changes Oct 9, 2025
Copy link
Collaborator

@Tishj Tishj left a comment

Choose a reason for hiding this comment

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

Thanks, looks great 👍

@paultiq
Copy link
Contributor

paultiq commented Oct 9, 2025

  • Randomized testing & Xdist: these need Test Order & Concurrency #75. They've been stable in my branch, tho, after fixing the test concurrency issues. The random hanging is probably the issues with query_interrupt.

@paultiq
Copy link
Contributor

paultiq commented Oct 9, 2025

Regarding ccache:

  • ccache action won't work (without additional changes) with CIBW's manylinux container, the build is running inside the container... so you'll have to move the cache in and out.
  • The reason I like sccache is that it caches on a per-object basis, which results in less cache churn. With ccache, you're writing an entire new artifact each time. This additional cache churn becomes a problem with many PRs each of which chipping away at the 10GB GHA cache limit.
  • add "ccache -s" at the end of the run, to confirm the cache hits. This probably should go inside cibw-before-test. I went through a series of steps to stabilize the directory paths... CIBW uses random build dirs which defeat caching without additional steps.
  • edit: Also, debugging this was a pain. Enabling verbose builds with build.tool-args = ["-v", "-d", "stats"] and equivalent for msvc / windows was necessary - need to make sure the full compile paths are stable across runs.

@evertlammerts
Copy link
Collaborator Author

Builds are passing. Getting pytest to respect config in pyproject.toml across all platforms was not much fun, but it works now.

I've removed the xdist plugin for now because tests became too flaky in CI. There is likely a lot left to win in our testing and CI in general, but other things need attention first.

@evertlammerts evertlammerts marked this pull request as ready for review October 10, 2025 08:55
@paultiq
Copy link
Contributor

paultiq commented Oct 10, 2025

One other test related suggestion, while you're in there; add --durations=5 to your pytest, which adds a report of the 5 slowest tests.

This is helpful for identifying any tests that shouldn't be in tests/fast.

@evertlammerts evertlammerts merged commit d16cffa into duckdb:v1.4-andium Oct 10, 2025
18 of 19 checks passed
@paultiq
Copy link
Contributor

paultiq commented Oct 10, 2025

I know you merged this, but one more comment related to this;

Should also add 3.14 to the pypi classifiers in pyproject.toml.

@evertlammerts
Copy link
Collaborator Author

Should also add 3.14 to the pypi classifiers in pyproject.toml.

Good find 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants