Full porting to astrapy 1.3+ #42
Conversation
…ding_api_key to accept EmbeddingHeadersProvider instances; +testing
| bulk_delete_concurrency=kwargs.get("bulk_delete_concurrency"), | ||
| collection_vector_service_options=collection_vector_service_options, | ||
| ) | ||
| return cls(**kwargs) |
There was a problem hiding this comment.
I think this will crash if we get an unknown argument
There was a problem hiding this comment.
That is sort of the intention - i.e. so that faulty invocations of _from_texts and _afrom_texts behave as if the caller were calling the (actual) constructor. Including the penalties for passing unknown arguments.
Would you prefer to be more prescriptive here?
There was a problem hiding this comment.
The current behavior is to ignore unknown parameters and that's also what the warning log says.
We can change to reject completely. Then the log would probably not be needed anymore.
There was a problem hiding this comment.
Ah, very well spotted! I just reintroduced the "warn but proceed" behaviour for the sake of maintaining current behaviour (for as little a corner case this might be...).
There was a problem hiding this comment.
Also added tests specific to this
|
LGTM, great work ! |
… 'thread'-related constants
… warning catch in unit tests
…tolerant handling of spurious kwargs in [a]from_texts 'init' methods
This is a large rewrite of the internals in order to completely migrate to using the recent, idiomatic astrapy.
Motivation
Guide to the PR
All the newest astrapy things are now usable (e.g. environments, TokenProvider, EmbeddingHeaderProvider classes and so on).
Along the way, several improvements came along:
Notes
vector_store.collection) would stop working. Such usage, anyway, is brittle in the first place and should not be encouraged.nb_prefetchedparameter, which however is ignored - actually a warning is emitted if a prefetch is requested. This reflects the fact that prefetched finds have been found to be problematic in some corner cases on AstraPy and are at the moment disabled. Functionally, this has no effect on the document loader class.