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

Controller search_documents function should accept multiple filter values for a single key #567

Closed
karimjp opened this issue Nov 8, 2020 · 2 comments · Fixed by #568
Closed
Assignees

Comments

@karimjp
Copy link
Contributor

karimjp commented Nov 8, 2020

Using the rest api I would like to make a request with multiple values for a filter key in the HTTP body to use them as filters on the retrieved results from my document store. The following two lines below in the Question class and search_documents function respectively doesn't seem to handle this case.

filters: Optional[Dict[str, Optional[str]]] = None

filters = {key: [value] for key, value in question_request.filters.items() if value is not None}

The first issue in the filters property in the Question class restricts the type to be a dictionary of key string and value string or null. In the case where the HTTP body has a list of values for a key filter name then it fails:

HTTP Body:
{ "questions": [ "is this a question?" ], "filters": {"key-1":["a","b"]}, "top_k_reader": 6, "top_k_retriever": 6 }
Response:
{ "detail": [ { "loc": [ "body", "filters", "key-1" ], "msg": "str type expected", "type": "type_error.str" } ] }

The second issue in the search_documents function is that regardless of filters being type str or List, each key of filters in line 181 needs to have a one dimensional list value.

@tanaysoni
Copy link
Contributor

Hi @karimjp, thank you for raising the issue. It should now be resolved with #568. Let me know if that works for your use case.

@karimjp
Copy link
Contributor Author

karimjp commented Nov 9, 2020

@tanaysoni looks and works great! Thank you for the quick turnaround.

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 a pull request may close this issue.

2 participants