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

Some smaller updates #27

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Some smaller updates #27

merged 4 commits into from
Jun 6, 2024

Conversation

morgangallant
Copy link
Contributor

@morgangallant morgangallant commented Jun 5, 2024

  • Includes a new test for bm25 row-based upsert with schema
  • Updates the type hint for attributes from Optional[str] to Optional[str | int], we support numerical attribute types

@morgangallant morgangallant requested a review from fw42 June 5, 2024 21:12
@morgangallant morgangallant changed the title add test for specifying schema in row-based upsert Some smaller updates Jun 5, 2024
@@ -175,7 +175,7 @@ def created_at(self) -> Optional[datetime]:
def upsert(self,
ids: Union[List[int], List[str]],
vectors: List[List[float]],
attributes: Optional[Dict[str, List[Optional[str]]]] = None,
attributes: Optional[Dict[str, List[Optional[str | int]]]] = None,
Copy link
Member

Choose a reason for hiding this comment

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

| isn't supported for union types on python 3.9, need to use Union[]

[
tpuf.VectorRow(id=8, vector=[0.8, 0.8], attributes={ "blabla": "row based upsert format is cool" }),
],
schema=schema,
Copy link
Member

Choose a reason for hiding this comment

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

Even though this seems to work, we should also make sure all the overrides have the schema in the parameter list

@fw42
Copy link
Contributor

fw42 commented Jun 6, 2024

Good catch. Sorry I missed this.

Signed-off-by: Morgan Gallant <[email protected]>
Signed-off-by: Morgan Gallant <[email protected]>
@morgangallant morgangallant force-pushed the mg/test-row-based-bm25-upsert branch from 4a66ece to 4c56e97 Compare June 6, 2024 14:55
Signed-off-by: Morgan Gallant <[email protected]>
@morgangallant
Copy link
Contributor Author

@pushrax went ahead and bumped version to v0.1.13 in here, mind merging this + doing the publish after final review? Added the schema param to the upsert helpers too, I don't think this changed anything, but I agree it's the right thing to do!

@pushrax pushrax merged commit 7613712 into main Jun 6, 2024
6 checks passed
@pushrax pushrax deleted the mg/test-row-based-bm25-upsert branch June 6, 2024 15:38
@pushrax
Copy link
Member

pushrax commented Jun 6, 2024

published

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants