-
Notifications
You must be signed in to change notification settings - Fork 10
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
#370 Added simple_search_query for concepts #345
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a way of running this code to see how the results have changed from the previous _build_concepts_query
query, but I like this approach! Once this is deployed we should try running searches with and without double-quotes in them so we can compare how _build_concepts_query()
compares with get_simple_search_query()
; if the latter is better overall (as I hope it will be!), we can get rid of the former entirely.
I have a few inline suggestions to make, but nothing that should hold this PR up assuming it works.
src/dug/core/async_search.py
Outdated
@@ -690,3 +418,134 @@ async def search_kg(self, unique_id, query, offset=0, size=None, | |||
) | |||
search_results.update({'total_items': total_items['count']}) | |||
return search_results | |||
|
|||
def get_query(self, concept, fuzziness, prefix_length, query): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to have some documentation on what this function does -- in particular, how it differs from _build_concepts_query()
, which looks very similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will rename and add a comment. It will add some clarity. Maybe someone else could provide more insight in differences, i don't have this info yet
} | ||
return query | ||
|
||
def get_simple_search_query(self, query): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this function should have some documentation with a reference to https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-simple-query-string-query.html.
@@ -621,17 +349,18 @@ async def search_vars_unscored(self, concept="", query="", | |||
"name": elem_s['element_name'] | |||
} | |||
|
|||
if scored: | |||
elem_info["score"]: round(elem['_score'], 6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need scores to 6 digits of precision? I would think 3 or 4 should be enough, but I don't have a strong opinion on this. Might be worth discussing in another ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are right, but i did not author this line of code, just moved it.
@gaurav Thank you for your comments. I addressed some in my next commit. |
No description provided.