fix(python): keep embedding functions registered without an alias readable - #4356
Open
MohammadHijjawi97 wants to merge 1 commit into
Open
MohammadHijjawi97 wants to merge 1 commit into
MohammadHijjawi97 wants to merge 1 commit into
Conversation
…dable `EmbeddingFunctionRegistry.register()` looks the class up under its class name when no alias is given, but stored `None` as the name that gets written into the table metadata. Tables using such a function were written with `"name": null`, and reading their embedding functions back failed with `KeyError: None`, which broke `add()` and `search()` on the table. Store the key the class is registered under instead, which is what the TypeScript registry already does.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
Storing the resolved registration key restores the documented class-name round-trip while preserving explicit aliases. The regression and local metadata checks verified reopening, embedding generation, text search, and reading newly written metadata with the previous parser.
This branch has not been deployed
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.
EmbeddingFunctionRegistry.register()documents that the class name is used when no alias is given, and it does register the class under that name. But it stored the alias itself (None) as__embedding_function_registry_alias__, which is whatfunction_to_metadata()writes into the table schema. A table using such a function gets"name": nullin itsembedding_functionsmetadata, and any later read of the functions fails:This stores the key the class is registered under instead, so the class name round-trips. For aliased classes the key is the alias, so their metadata is unchanged. The TypeScript registry already falls back to
ctor.namein the same way.Added
test_embedding_function_registered_without_alias, which creates a table with such a function, reopens it, and adds a row. It fails withKeyErrorbefore the change.test_embeddings.py,test_pydantic.py,test_table.pyandtest_db.pypass, andruff format --check/ruff checkare clean.