Fixing broken BM25 support with Weaviate - fixes #3720

[zoltan-fedor]

Unfortunately the BM25 support with Weaviate got broken with Haystack v1.11.0+, which is getting fixed with this commit.

Please see more details under issue #3720.

Related Issues

• fixes BM25 support with Weaviate got broken with Haystack v1.11.0+ #3720

Proposed Changes:

The issue was caused by this commit.

I have fixed it by:

• making the Weaviate document store to be a subclass of KeywordDocumentStore
• adding the query_batch() method to the Weaviate document store, although there is no batch querying in Weaviate currently, see @laura-ham 's comment from Weaviate, so the query_batch() will be implemented as a loop (just like it is for some other document stores in Haystack which don't support batch querying).
• adding Weaviate to the BM25 node tests, so this can't happen again

How did you test it?

1. Expanded the test_retrieval_without_filters test case to include Weaviate with BM25
2. I also had a local project using BM25 with Weaviate, so I have also tested this change via that project to ensure that it in fact works with real data.

Notes for the reviewer

The BM25 search WITH filters is still not supported in Weaviate, that is coming with Weaviate v1.18.0, at which point additional tests will need to be allowed to run for Weaviate (the ones with "filters")

Checklist

• [ x ] I have read the contributors guidelines and the code of conduct
• [ x ] I have updated the related issue with new insights and changes
• [ x ] I added tests that demonstrate the correct behavior of the change
• [ x ] I've used the conventional commit convention for my PR title
• [ x ] I documented my code
• [ x ] I ran pre-commit hooks and fixed any issue

[ZanSara]

LGTM! Thank you for taking care of WeaviateDocumentStore 😊

[zoltan-fedor]

Hi @ZanSara,
The query() method of the WeaviateDocumetnStore no longer has a default value for the query param, as its signature now must match the KeywordDocumentStore's query() abstractmethod's signature.
That is why the query=None has been added to those test. All that is to match the signature of the KeywordDocumentStore's abstractmethods.
That is why the test is breaking now after your commit.

I agree, that the KeywordDocumentStore's query() abstractmethod's signature should probably have None as default value for the query param, but it doesn't have it and I didn't want to modify the abstractmethod. I personally believe that should be changed, but being an abstractmethod I didn't want to go there, as then all other methods which already implemented this would also needed to be changed.

[ZanSara]

Hey @zoltan-fedor, you're right, I didn't notice you adjusted the default. Let me revert.