-
Notifications
You must be signed in to change notification settings - Fork 10
Conversation
078c2c1
to
6d9041f
Compare
0bb790e
to
c9eb79c
Compare
idunn/api/urls.py
Outdated
APIRoute( | ||
"/search", | ||
search, | ||
response_model=IdunnAutocomplete, | ||
), |
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.
For consistency with "/autocomplete", methods
and response_model_exclude_unset
should be defined too.
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.
... and we should allow to provide extra parameters through POST queries 👍
idunn/api/search.py
Outdated
""" | ||
query.q = normalize(query.q) | ||
|
||
if query.lang in nlu_allowed_languages: |
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.
if query.lang in nlu_allowed_languages: | |
if query.nlu and query.lang in nlu_allowed_languages: |
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.
We also want to keep this option overridden by AUTOCOMPLETE_NLU_SHADOW_ENABLED, right?
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.
Well, this option has never been actually used, and is probably not very useful now that the nlu=true
is used by the application in most cases.
So I think this flag AUTOCOMPLETE_NLU_SHADOW_ENABLED
can be ignored for /search
.
8d8c527
to
e410d04
Compare
c5a5a99
to
5cdc530
Compare
This mostly reuses code from
instant_answer
but I tried to change the API a bit to be more consistent with what we usually use internally between erdapfel and Idunn:get_place
(through keyplace
) or the format ofget_bbox_places
(through keybbox_places
)The only slight behavior change is that there is no truncating of the input query, maybe we should add a global parameter for this? I've also thought about skipping normalization since it would seem weird to "ignore or change part of what is in the search bar" but this seems like a free potential improvement of results?
Part of the logs may be weird with the current state of the branch, I'm still thinking there may be some global pattern that could be built since fetching the context from the logger is a common issue we have (eg. how we build extra infos in
nlu_client
or in this case understanding what endpoint is currently being used).