fix(embeddings): check for numpy once per response - #3754
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b175a3cce4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if not obj.data: | ||
| raise ValueError("No embedding data received") | ||
|
|
||
| use_numpy = has_numpy() |
There was a problem hiding this comment.
Defer the NumPy probe until decoding is needed
When an OpenAI-compatible endpoint ignores the automatically requested base64 format and returns only ordinary float vectors, this unconditional call now imports the optional NumPy package even though every item is skipped below. Previously has_numpy() was never called for such a response, so an installed NumPy package that fails during import with a non-ImportError can now prevent an otherwise valid float response from being returned, and even a healthy first import adds unnecessary latency. Cache the result lazily when the first string vector is encountered instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, that was a real behavior change. If an endpoint ignores the base64 request and sends plain float vectors, the old code never reached has_numpy() at all, and my hoist made it run on every response.
c519bd1 moves the check to the first string vector and caches it in the local, so a base64 response still checks once and a float-only response never checks. Added a test that fails a decoder call for a response with no string vectors.
Changes being requested
parse_embedding_responsecallshas_numpy()once per embedding, and that function does a freshimport numpyevery time. Python does not cache failed imports, so when numpy isn't installed each call walks the path finders again and builds an ImportError. A 1500-embedding response pays that 1500 times. Hoisting the check above the loop keeps the behaviour identical and does the import work once.Decoding 1500 vectors without numpy goes from 96ms to 63ms on my machine, where
sys.pathhas six entries. The reporter is on Windows and sees the same call take seconds.src/openai/lib/is hand-written and the generator does not touch it, per CONTRIBUTING.md.Additional context & links
Fixes #3753