Add more docstrings for ElasticsearchDocumentStore and ElasticsearchBM25Retriever
#184
Conversation
silvanocerza
requested review from
shadeMe and
TuanaCelik
and removed request for
a team
| :type filters: Optional[Dict[str, Any]], optional | ||
| :param fuzziness: Fuzziness parameter passed to Elasticsearch, defaults to "AUTO". | ||
| see the official documentation for valid values: | ||
| https://www.elastic.co/guide/en/elasticsearch/reference/current/common-options.html#fuzziness |
There was a problem hiding this comment.
Shouldn't we be following the RST syntax?
There was a problem hiding this comment.
Now that I think about it this should be valid RST syntax, the link will be highlighted and threated as such. 🤔
There was a problem hiding this comment.
I'll take your word for it 👍
That being said, we might want to discuss elsewhere if using hyperlinks (as opposed to bare URLs) improves readability. And on a related note, we should also add a CI workflow that automatically deploys documentation changes to a preview service
| :param document_store: An instance of ElasticsearchDocumentStore. | ||
| :type document_store: ElasticsearchDocumentStore | ||
| :param filters: Filters applied to the retrieved Documents, for more info | ||
| see `ElasticsearchDocumentStore.filter_documents`, defaults to None |
There was a problem hiding this comment.
Sphinx should automatically generate the default value, right? (Same question everywhere else).
There was a problem hiding this comment.
As far as I know it doesn't, and I can't find anything about it either to be fair. 🤔
There was a problem hiding this comment.
I know that it has support for it having used it in previous projects, but it might be something that needs to be configured/theme-dependent.
If we currently always specify the default value in the docstring, we'll leave the defaults in this PR. But in that case, we should open an issue to rectify this globally.
There was a problem hiding this comment.
As of now we specify it, same thing in the Haystack core repo. I totally agree that we need more standardization on the documentation side.
Tagging @dfokina to keep her in the loop.
Solves deepset-ai/haystack#6601