Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IndexSchema refactor and async collapse #85

Merged
merged 37 commits into from
Dec 20, 2023
Merged

Conversation

tylerhutcherson
Copy link
Collaborator

@tylerhutcherson tylerhutcherson commented Nov 28, 2023

This PR introduces a new user space class, IndexSchema. This effort to refactor the index schema is key for us to be able to enable "wrappability" and extensibility of RedisVL with other integration efforts (i.e. LangChain), and also improve the UX of working with the tool overall.

A few changes:

  • SearchIndex now expects the schema to be provided on init, or built with some of the constructors (common).
  • SearchIndex now optionally expects redis_url or redis_client
  • An initial MVP of the RedisConnection class was built to support working with both sync and async connections from redis-py. A better implementation will be coming in the near term.
  • Async and Sync search index were combined. No more AsyncSearchIndex class here.
  • Other updates and docs improvements.

Adding link to current notes on the schema spec: https://docs.google.com/document/d/1vkShf9PoQd1FnjYa2PpKhcgk8FW9WZGi_KpCuoDtvEY/edit?usp=sharing

Copy link
Contributor

@Spartee Spartee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start. Lots of thoughts on here. Would be good to sync on this and what you're doing with collapsing the interfaces.

Biggest theme here is reduction in code duplication, I think we can collapse

  • SchemaModel
  • FieldsModel
  • SchemaGenerator

into

  • Schema

Let me know what you think. Great start!

redisvl/schema/fields.py Outdated Show resolved Hide resolved
redisvl/schema/schema.py Outdated Show resolved Hide resolved
redisvl/schema/schema.py Outdated Show resolved Hide resolved
redisvl/schema/schema.py Outdated Show resolved Hide resolved
redisvl/schema/__init__.py Outdated Show resolved Hide resolved
redisvl/index.py Outdated Show resolved Hide resolved
redisvl/index.py Show resolved Hide resolved
redisvl/index.py Outdated Show resolved Hide resolved
redisvl/index.py Outdated Show resolved Hide resolved
redisvl/index.py Outdated Show resolved Hide resolved
redisvl/schema/schema.py Outdated Show resolved Hide resolved
@tylerhutcherson tylerhutcherson changed the base branch from main to dev December 15, 2023 20:22
@tylerhutcherson tylerhutcherson changed the title Schema updates and specification IndexSchema refactor and async collapse Dec 15, 2023
@tylerhutcherson tylerhutcherson added the enhancement New feature or request label Dec 15, 2023
@tylerhutcherson tylerhutcherson marked this pull request as ready for review December 15, 2023 20:26
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2023

Codecov Report

Attention: 63 lines in your changes are missing coverage. Please review.

Comparison is base (9b870ca) 80.54% compared to head (e6e4b01) 78.64%.

Files Patch % Lines
redisvl/utils/utils.py 15.38% 22 Missing ⚠️
redisvl/schema/schema.py 87.12% 17 Missing ⚠️
redisvl/utils/connection.py 72.22% 10 Missing ⚠️
redisvl/index.py 88.40% 8 Missing ⚠️
redisvl/llmcache/semantic.py 86.11% 5 Missing ⚠️
redisvl/schema/fields.py 98.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev      #85      +/-   ##
==========================================
- Coverage   80.54%   78.64%   -1.91%     
==========================================
  Files          19       21       +2     
  Lines        1203     1283      +80     
==========================================
+ Hits          969     1009      +40     
- Misses        234      274      +40     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tylerhutcherson tylerhutcherson merged commit da9c57a into dev Dec 20, 2023
18 checks passed
@tylerhutcherson tylerhutcherson deleted the schema-updates branch December 20, 2023 16:49
Spartee pushed a commit that referenced this pull request Jan 19, 2024
Introduce the `IndexSchema`.
---------
Co-authored-by: Sam Partee <sam.partee@redis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants