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

feature: fetch_objects_by_ids #1241

Merged
merged 7 commits into from
Aug 20, 2024

Conversation

tibor-reiss
Copy link
Contributor

@tibor-reiss tibor-reiss commented Aug 14, 2024

Fixes #1168

@tibor-reiss
Copy link
Contributor Author

@tsmith023 Is generate.fetch_objects_by_ids also needed? I removed it in the 2nd commit for now.

@tsmith023
Copy link
Contributor

tsmith023 commented Aug 14, 2024

Hi @tibor-reiss, thank you so much for this contribution!

I don't think implementing generate.fetch_objects_by_ids is necessary. We do have generate.fetch_objects but in hindsight this method isn't actually particularly useful; I wonder how many people really use it 🤷

The linting check is failing with a black error. You will need to install the version of black defined in requirements-devel.txt and run it on the codebase to fix it!

black weaviate test mock_tests integration

@dirkkul
Copy link
Collaborator

dirkkul commented Aug 15, 2024

I don't think implementing generate.fetch_objects_by_ids is necessary. We do have generate.fetch_objects but in hindsight this method isn't actually particularly useful; I wonder how many people really use it 🤷

generate.fetch_objects_by_ids might actually make sense here if you only want to generate something for a few IDs. But that could be done in a separate PR

@weaviate-git-bot
Copy link

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

@tibor-reiss
Copy link
Contributor Author

@weaviate-git-bot I agree with the CLA

@tibor-reiss
Copy link
Contributor Author

I don't think implementing generate.fetch_objects_by_ids is necessary. We do have generate.fetch_objects but in hindsight this method isn't actually particularly useful; I wonder how many people really use it 🤷

generate.fetch_objects_by_ids might actually make sense here if you only want to generate something for a few IDs. But that could be done in a separate PR

I added back the generate part.

@dirkkul dirkkul merged commit e974108 into weaviate:main Aug 20, 2024
49 of 50 checks passed
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.

Add .fetch_objects_by_ids() method as shorthand for filtering with .fetch_objects
4 participants