Conversation
frayle-ons
left a comment
frayle-ons
left a comment
There was a problem hiding this comment.
LGTM, one small change requested - and can we confirm this will work okay with quartodocs.
src/classifai/indexers/main.py
Outdated
| self.hooks = {} if hooks is None else hooks | ||
| self.vectoriser_class = vectoriser.__class__.__name__ | ||
| ## these are all to be filled in from vectors creation | ||
| self.vector_shape: int | None = None |
There was a problem hiding this comment.
are self.num_vectors andself.vector_shape the same as self.vectors in this case? they are defined later so why not just remove the None logic from here and let lines 209 and 210 do the assignment?
There was a problem hiding this comment.
Can remove these, quarto seems to build fine but it doesn't seem to show constructors, wondering if we'd want to configure for this?
src/classifai/indexers/main.py
Outdated
| file_name (str | os.PathLike): The name of the input CSV file. | ||
| data_type (str): The type of input data (currently supports only "csv"). | ||
| vectoriser (object): The vectoriser object used to transform text into | ||
| vectoriser (vectoriserBase): The vectoriser object used to transform text into |
There was a problem hiding this comment.
small note: [V]ectoriserBase (capitalisation)
|
This all looks good to me - but before merging, can we check that initialising a VectorStore with a CSV within a GCP Bucket still works with the new checks added? (I know we haven't advertised that functionality yet, but it would be a shame to break it) |
3 participants
✨ Summary
Fixes the typing for vectorstore to add typehints to the methods and resolve type errors with internal
.vectorsAlso fixes bug with deptry on pre-commit hook
Also updates server pydantic models to patch out deprecated extra definition (now uses configdict)
Resolves #142
📜 Changes Introduced
adds in correct entrypoint for deptry in pre-commit docker
adds proper init typehints for vectorstore
adds support for pathlike for filename inputs
Feature implementation (feat:) / bug fix (fix:) / refactoring (chore:) / documentation (docs:) / testing (test:)
Updates to tests and/or documentation
Terraform changes (if applicable)
✅ Checklist
terraform fmt&terraform validate)🔍 How to Test
Run usual demo to check if it inits correctly, then try inputting in Path as file location, check if intellisense looks better