bindings/python: align max_connections default with engine #5352 (16 -> 32) + document Vamana semantics - #5355
bindings/python: align max_connections default with engine #5352 (16 -> 32) + document Vamana semantics#5355tae898 wants to merge 1 commit into
Conversation
…-20) Regenerated by make-upstream-pr-branch.sh: bindings/python brought to the fork's main state. Scope: the minimum needed to reproduce the wheel build and run the tests and examples.
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Code Review
This pull request updates the Python bindings and examples. It replaces references to the "real_ladybug" package with "ladybug" in the StackOverflow OLTP and OLAP examples. Additionally, it increases the default value for "max_connections" (or "maxConnections") from 16 to 32 across vector index building, hybrid queries, and the core embedded API, updating the corresponding documentation and help messages to reflect this change and explain its relation to JVector/Vamana per-layer degree. There are no review comments, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
Checking in on this one, and asking a question rather than just nudging. State as I read it: mergeable, one commit, The question. We have a second Measured on TSBS, released 26.8.1.dev21, mini, N=3:
The 3.09x was our own adapter's fault and is already fixed on our side: it was passing Python lists, taking the per-element conversion path instead of the bulk array path the binding has always had. The 1.34x on top is your new primitive So: separate PR against |
|
Thanks for the follow-up, @tae898, and thanks especially for catching that the wrapper was shadowing the engine default. That one was easy to miss: A few things I'd like to work through with you before we merge. Example 11 and the shared
|
|
@tae898 rather than leave this sitting, I went ahead and did the follow-ups myself. They are in #5617, which keeps your original commit and authorship intact and adds five commits on top. I tried to push them straight to this branch first, since maintainer edits are enabled. That does not work here: What I added:
I also dropped the docs paragraph from the description, since those files live only in your fork. If you would rather land this yourself, say so and I will close #5617 and hand the patch back, or you can grant push access on the fork and I will move the commits over. Happy to take either. Thanks again for catching the shadowed default, which was a genuine bug that would have quietly halved base-layer density for every Python user who did not pass the argument. On your TSBS question from the 29th, that deserves its own thread and I will reply there separately. |
|
Superseded by #5617, which is merged. It carries this commit with its authorship intact, plus the degree-matching fix for example 11, a regression test that reads Closing in favour of that. The docs updates named in the description above live in |
Follows up #5352: the Python wrapper (
create_vector_index) carried its ownmax_connections=16default, now inconsistent with the engine's new default of 32. This aligns the wrapper and updates all bindings docs/examples with the documented semantics (per-layer Vamana degree, not hnswlib M; use 2*M to reproduce an hnswlib configuration).🤖 Generated with Claude Code
https://claude.ai/code/session_01KPPtpXXDs5EZFmx9DAPnRB