chore(python): python 3.14 support#7728
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes update Python CI and tooling from 3.13 to 3.14, refresh test dependency constraints, and make background executor pump errors version-dependent. ChangesPython 3.14 validation updates
Executor pump error propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
A-python
|
ACTION NEEDED The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/pyproject.toml`:
- Line 106: Align the Pyright target with the project’s supported baseline by
changing the pythonVersion setting in pyproject.toml to 3.10, matching
requires-python >=3.10; alternatively, add a separate check that type-checks
against the minimum supported Python version.
- Around line 75-76: Align the compatibility job with the pinned test
dependencies by either adding a shared exported constraints/lockfile and
installing it in the workflow’s pylance[tests,ray] job, or updating
[project.optional-dependencies].tests to mirror the datasets==4.4.0 and
duckdb>=1.5.0,<1.6.0 constraints shown in pyproject.toml.
In `@python/src/executor.rs`:
- Around line 239-243: The public API rustdoc for block_on_pumping must document
that completion-time draining of buffered progress events can return a PyErr
even after the future succeeds. Update its documentation with an executable
example demonstrating final pump/drain behavior and PyErr propagation, linking
to the relevant executor methods and types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf85f057-17e8-412f-9e37-6218e023ad9d
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/python.ymlpython/pyproject.tomlpython/src/executor.rs
|
|
||
| [tool.pyright] | ||
| pythonVersion = "3.13" | ||
| pythonVersion = "3.14" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== python/pyproject.toml (around line 106) ==\n'
nl -ba python/pyproject.toml | sed -n '1,180p'
printf '\n== Search for pyright config and requires-python ==\n'
rg -n --hidden -S 'pythonVersion|requires-python|pyright' python . -g '!**/.git/**'Repository: lance-format/lance
Length of output: 243
🏁 Script executed:
set -euo pipefail
printf '\n== python/pyproject.toml (excerpt) ==\n'
awk 'NR>=1 && NR<=180 { printf "%4d %s\n", NR, $0 }' python/pyproject.toml
printf '\n== Search for pyright/min-version settings ==\n'
rg -n --hidden -S 'pythonVersion|requires-python|pyright|minimum version|min-version|3\.14|3\.10' python .Repository: lance-format/lance
Length of output: 50375
🏁 Script executed:
set -euo pipefail
printf '\n== python/pyproject.toml (lines 90-120) ==\n'
sed -n '90,120p' python/pyproject.toml | awk '{ printf "%4d %s\n", NR+89, $0 }'
printf '\n== config files mentioning pythonVersion/requires-python ==\n'
fd -HI -t f 'pyproject.toml|pyrightconfig.json|*.json|*.toml|*.yaml|*.yml' python . | head -n 50
printf '\n== exact matches in python/ directory ==\n'
rg -n --hidden -S '^(pythonVersion|requires-python)\s*=|pythonVersion|requires-python' python/pyproject.toml python/pyrightconfig.json python/.github python/.circleci python/.pre-commit-config.yaml python/.pre-commit-config.yml python/pyrightconfig.jsonRepository: lance-format/lance
Length of output: 1551
🏁 Script executed:
set -euo pipefail
printf '\n== python/pyproject.toml (lines 90-120) ==\n'
awk 'NR>=90 && NR<=120 { printf "%4d %s\n", NR, $0 }' python/pyproject.toml
printf '\n== likely config files under python/ ==\n'
find python -maxdepth 2 \( -name 'pyproject.toml' -o -name 'pyrightconfig.json' -o -name '*.toml' -o -name '*.json' -o -name '*.yml' -o -name '*.yaml' \) | sort
printf '\n== exact matches in python config files ==\n'
rg -n --hidden -S 'pythonVersion|requires-python' python/pyproject.toml python/pyrightconfig.json python/.pre-commit-config.yaml python/.pre-commit-config.yml python/.github python/.circleci 2>/dev/null || trueRepository: lance-format/lance
Length of output: 1395
🏁 Script executed:
set -euo pipefail
printf '\n== python/pyproject.toml (lines 90-120) ==\n'
awk 'NR>=90 && NR<=120 { printf "%4d %s\n", NR, $0 }' python/pyproject.toml
printf '\n== matches for pythonVersion/requires-python under python/ ==\n'
rg -n --hidden -S 'pythonVersion|requires-python' pythonRepository: lance-format/lance
Length of output: 1324
Align Pyright with the supported baseline python/pyproject.toml:105-106
requires-python is still >=3.10, but pythonVersion = "3.14" makes Pyright type-check against 3.14 syntax/stubs, so 3.10–3.13 compatibility issues can slip through. Set this to 3.10 or add a separate minimum-version check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/pyproject.toml` at line 106, Align the Pyright target with the
project’s supported baseline by changing the pythonVersion setting in
pyproject.toml to 3.10, matching requires-python >=3.10; alternatively, add a
separate check that type-checks against the minimum supported Python version.
There was a problem hiding this comment.
I think this is a valid point. We should probably set pythonVersion to 3.10 for linting. I would keep this as is for consistency for now.
There was a problem hiding this comment.
@mocobeta Understood, thanks for confirming. I'll leave this as-is for now since you're intentionally prioritizing consistency with the CI baseline. Feel free to revisit this when you're ready to formalize the linting target at the minimum supported version (3.10).
✏️ Learnings added
Learnt from: mocobeta
Repo: lance-format/lance PR: 7728
File: python/pyproject.toml:106-106
Timestamp: 2026-07-11T16:30:06.317Z
Learning: In `python/pyproject.toml`, the `[tool.pyright]` `pythonVersion` setting is intentionally kept aligned with the latest supported Python version (currently 3.14) for consistency, even though `requires-python` allows a lower minimum (`>=3.10`). This is a known/accepted inconsistency; don't flag it again unless asked.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| // when the index build finishes so fast that no pump cycles | ||
| // occurred during execution (common on Python 3.14 due to | ||
| // GIL/async scheduling changes), all events sit in the | ||
| // channel buffer and get drained after completion | ||
| pump()?; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the completion-time pump error contract.
pump()? now turns a successful future into an error when draining buffered progress events fails, but the block_on_pumping rustdoc does not describe this behavior or provide an executable example. Update the public API documentation to show final buffered-event draining and PyErr propagation, with links to the relevant methods.
As per coding guidelines, **/*.{rs,md,py} requires all public APIs to have documentation with examples, and documentation should link to relevant structs and methods.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/src/executor.rs` around lines 239 - 243, The public API rustdoc for
block_on_pumping must document that completion-time draining of buffered
progress events can return a PyErr even after the future succeeds. Update its
documentation with an executable example demonstrating final pump/drain behavior
and PyErr propagation, linking to the relevant executor methods and types.
Source: Coding guidelines
| strategy: | ||
| matrix: | ||
| python-minor-version: ["10", "13"] | ||
| python-minor-version: ["10", "14"] |
There was a problem hiding this comment.
We should probably have at least one CI job that also runs on 3.13 in addition to 3.10 and 3.14 to verify we don't break support for it? Fortunately, 3.10 will be EOL here pretty soon.
| } | ||
| // when the index build finishes so fast that no pump cycles | ||
| // occurred during execution (common on Python 3.14 due to | ||
| // GIL/async scheduling changes), all events sit in the | ||
| // channel buffer and get drained after completion | ||
| pump()?; |
There was a problem hiding this comment.
I'm sort of confused by this change. Why is propagating this error related to GIL changes?
There was a problem hiding this comment.
Sorry I forgot to add the context of this change in the PR description.
I found that this test failed with Python 3.14. The expected RuntimeError was not thrown.
Honestly, I am not an expert in GIL-free Python 3.14. My coding agent detected the possible cause of the test failure. It says the expected error in the test was silently logged in the block instead of being properly propagated.
I'm copy-pasting the coding agent's report.
If Python 3.14 changed async scheduling such that all progress events now arrive after the index build future completes (rather than interleaved during execution), errors would be silently logged instead of propagated. This could be caused by:
- GIL scheduling changes in Python 3.14 affecting when
Python::try_attachcan re-attach to the Python interpreter - Tokio event loop timing differences — the index build may now complete too quickly for pump cycles to interleave with stage events
- A race condition where the channel fills but drain() only runs after completion
This is likely a real lance code bug exposed by Python 3.14's threading/timing changes, not just a test issue. The fix should be on line ~248 of python/src/executor.rs — errors from post-completion pump cycles should propagate, not be logged as warnings (at least for non-complete events like "start:train_ivf").
There was a problem hiding this comment.
I did some additional research with Qwen3.6. The subtle timing change seems to occur with both GIL-enabled and disabled Python 3.14. (It seems the code was affected by an implementation-level change in CPython 3.14, but I myself cannot provide the rationale.)
Root cause
In block_on_pumping, progress events are drained via pump() at two points:
- During execution (
pump()?;) — errors propagate correctly - After completion (post-completion drain) — the GIL-enabled branch swallows errors with
log::warn!
On Python 3.14, async scheduling changes cause the index build to complete faster than before. No events exist in the channel during the first pump cycle, so they all get drained post-completion where the old code silently drops them:
There was a problem hiding this comment.
The test failure only happens with 3.14. I introduced a fallback to the current behavior when running on 3.13 or older.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/pyproject.toml (1)
106-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRun Pyright against the supported minimum version too.
requires-pythonstill supports Python 3.10, butpythonVersion = "3.14"makes this check model only the 3.14 environment. Add a separatepyright --pythonversion 3.10check (while retaining the 3.14 check), or target 3.10 here. Pyright documentspythonVersionas controlling version-specific analysis. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/pyproject.toml` at line 106, Update the Pyright configuration’s pythonVersion setting to ensure analysis also runs against the supported minimum Python 3.10 version, while retaining coverage for Python 3.14 through a separate check or equivalent configuration.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@python/pyproject.toml`:
- Line 106: Update the Pyright configuration’s pythonVersion setting to ensure
analysis also runs against the supported minimum Python 3.10 version, while
retaining coverage for Python 3.14 through a separate check or equivalent
configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2f3bb68-b43b-40a9-804d-697fbc516c6f
📒 Files selected for processing (1)
python/pyproject.toml
mocobeta
changed the title
|
Hi @westonpace, when I tried to run the benchmark scripts following |
Benchmark resultI ran the benchmark (#7744 patch applied) on my PC both with Python 3.13 and 3.14. Machine Spec:
Commands:Result:In the run, I observe slight performance degradation in 3.14, although my PC is not the latest 🤔 Just to note, performance-related interests are out of scope of this change. My intention here is to ensure the benchmark runs successfully with Python 3.14. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/src/executor.rs`:
- Around line 22-24: Update is_python314_or_later so a None argument first
checks the version of the currently attachable interpreter, returning whether it
is Python 3.14 or later; only use the existing fallback when no interpreter can
be attached. Preserve the direct version check for Some(py).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ad9259d-f8b8-4610-93d2-4fa921b485ba
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
python/src/executor.rs
| fn is_python314_or_later(py: Option<Python<'_>>) -> bool { | ||
| py.map_or(true, |py| py.version_info() >= (3, 14)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant Rust files first
ast-grep outline python/src/executor.rs --view expanded
ast-grep outline python/src/dataset.rs --view expanded
# Locate the helper and its call sites
rg -n "is_python314_or_later|block_on_pumping\(" python/src/executor.rs python/src/dataset.rs
# Read the relevant slices with line numbers
sed -n '1,120p' python/src/executor.rs
printf '\n==== dataset slice ====\n'
sed -n '4320,4405p' python/src/dataset.rsRepository: lance-format/lance
Length of output: 23019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the pumping implementation and nearby call sites
sed -n '191,285p' python/src/executor.rs
printf '\n==== all block_on_pumping call sites ====\n'
rg -n "block_on_pumping\(" -S python/srcRepository: lance-format/lance
Length of output: 3966
Handle None using the attachable interpreter version
python/src/dataset.rs calls block_on_pumping(None, ...), so this path currently treats every run as 3.14+ and propagates post-completion callback errors even on 3.13 and older. When py is None, check the currently attachable interpreter before choosing the branch, and only fall back when no interpreter is available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/src/executor.rs` around lines 22 - 24, Update is_python314_or_later so
a None argument first checks the version of the currently attachable
interpreter, returning whether it is Python 3.14 or later; only use the existing
fallback when no interpreter can be attached. Preserve the direct version check
for Some(py).
|
I fixed the clippy warning. |
2 participants
This PR adds Python 3.14 support.
duckdbanddatasetsexecutor.rsto properly propagate the runtime error with Python 3.14 (GIL-disabled)All tests passed with some warnings.
Benchmark result
Summary by CodeRabbit
Summary by CodeRabbit
Chores
Bug Fixes