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

Rename the QdrantRetriever + /retrievers folder #134

Closed
Tracked by #319
bilgeyucel opened this issue Dec 20, 2023 · 3 comments · Fixed by #174
Closed
Tracked by #319

Rename the QdrantRetriever + /retrievers folder #134

bilgeyucel opened this issue Dec 20, 2023 · 3 comments · Fixed by #174
Labels
feature request Ideas to improve an integration integration:qdrant

Comments

@bilgeyucel
Copy link
Contributor

Is your feature request related to a problem? Please describe.
As a user, I'd like to have predictable names for components and other retriever names contain the retrieval method

Describe the solution you'd like
Renaming the QdrantRetriever as QdrantEmbeddingRetriever to align with the current convention. Also, since we foresee QdrantSparseRetriever and QdrantHybridRetriever, it makes sense to create a new retrievers folder and move QdrantEmbeddingRetriever into it.

Describe alternatives you've considered
N/A

Additional context
N/A

@bilgeyucel bilgeyucel added feature request Ideas to improve an integration integration:qdrant labels Dec 20, 2023
@mathislucka mathislucka added this to the 2.0 DocumentStores milestone Dec 22, 2023
@sahusiddharth
Copy link
Contributor

sahusiddharth commented Dec 27, 2023

@bilgeyucel

Can you please confirm?

Things to do:-

  • Rename the current [QdrantRetriever](https://github.com/deepset-ai/haystack-core integrations/blob/main/integrations/qdrant/src/qdrant_haystack/retriever.py#L9) to QdrantEmbeddingRetriever.
  • Create a new folder named retriever inside src/qdrant_haystack and put retriever.py inside it.

@sahusiddharth
Copy link
Contributor

Hi @silvanocerza , I hope you're doing well. I noticed that the issue is still open. Could you please provide an update on its status?

@silvanocerza
Copy link
Contributor

@sahusiddharth this can be closed I think, I left it open cause it was also about adding a new retrievers folder but we've done that with #255.

I'll close it as done. Thanks for pinging. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Ideas to improve an integration integration:qdrant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants