fix(python): accept none dimension in VectorSchema - #774
Open
HosniBelfeki wants to merge 1 commit into
Open
HosniBelfeki wants to merge 1 commit into
HosniBelfeki wants to merge 1 commit into
Conversation
VectorSchema documents `dimension` as `Optional[int] = 0` and its docstring states the value "may be `None` for sparse vectors", but the constructor rejects None: `isinstance(None, int)` is False, so the guard raises "Invalid schema: vector's dimension must be >= 0" — a range error reported for what is really a type rejection. Normalize None to 0 before the range check, mirroring the normalization the same constructor already applies to its other Optional parameter (field_schema.py:251, `if index_param is None: index_param = FlatIndexParam()`). Sparse vector fields already use dimension 0; the repo's own fixture reaches it by omitting the argument (python/tests/detail/fixture_helper.py:78). test_dimension_none pins the documented contract and fails without this change with the ValueError above. test_dimension_invalid guards the range and type rejections the fix touches, and passes both with and without it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
VectorSchemadeclaresdimension: Optional[int] = 0, and its docstring statesthe value "may be
Nonefor sparse vectors"(python/zvec/model/schema/field_schema.py:204-205, 225).
The constructor rejects it.
isinstance(None, int)isFalse, so line 241 raises:Two problems: the documented and annotated
Nonedoes not work, and the errorreports a range violation for what is actually a type rejection.
Severity
Low, and not a security issue. Nothing is silently wrong and no data is
affected — a user who follows the documented sparse-vector idiom just hits a
confusing error. The repo's own fixture sidesteps it by omitting the argument
(python/tests/detail/fixture_helper.py:78), which is likely why it went unnoticed.
Fix
Normalize
Noneto0before the range check. This mirrors what the sameconstructor already does for its other
Optionalparameter, nine lines below:Nonehandlingindex_paramOptional[...] = NoneFlatIndexParam()(line 251)dimension(before)Optional[int] = 0ValueErrordimension(after)Optional[int] = 000is the value sparse vector fields already carry, so this changes no existingbehavior — inputs that worked before produce identical schemas. Dense fields
declared with dimension 0 are still rejected downstream by the C++ validator
(
field[...]'s dimension must be in (0,20000]), which is where the check belongs,since 0 is legal for sparse.
Tests
test_dimension_nonepins the documented contract. Without the fix it fails withValueError: Invalid schema: vector's dimension must be >= 0; with it, the schemabuilds and
dimension == 0.test_dimension_invalidguards what the fix touches —-1and"128"must stillbe rejected. It passes both with and without the change.
Verified on Linux (Debian 13 container, x86_64, CPython 3.12.14) and on Windows 11
(CPython 3.12.4).
pytest python/tests/test_schema.py→11 passed. Reverting thethree-line change while keeping the tests gives
1 failed, 10 passedon bothplatforms, failing with the
ValueErrorquoted above.ruff==0.14.4(CI's pin):ruff check .→All checks passed!,ruff format --check→2 files already formatted.