Merriam Webster Backend, Antonyms, and Result Caching#48
Merriam Webster Backend, Antonyms, and Result Caching#48ahayman wants to merge 42 commits intoRon89:testingfrom
Conversation
- Caching logic is finished - Renamed dictionary_api backend to merriam_webster for clarity - Cleaned up docs and code
…merriam-webster" This reverts commit 7cc2f0a.
There was a problem hiding this comment.
Hey, sorry for the extremely long delay. A lot in my life were evolving in the past year. so I neglected these repos that I should have been maintaining on GitHub. And look what I missed out here...
Thank you for this large and helpful PR. It is very nicely written and meaningfully broaden the framework. I would very much like to merge it into my repo. I did noticed some logic that may affect other features, so I made some suggestions. And I do believe they are necessary before I can pull these changes in. Could you help patching them up? Or if you think differently regarding those suggestions, please also comment down, we can discuss more.
| synonym_list=[] | ||
|
|
||
| # Check cache first | ||
| if not self.cached_used and cache_results > -1: |
There was a problem hiding this comment.
I have a question, if cache is used unconditionally... how does user request the thesaurus engine to query from a different backend (next backend in line, for example)? Wouldn't the logic stuck and always return from this cache?
There was a problem hiding this comment.
I am thinking of using a flag firstQuery that gets set to true in session_init(). And cache is only used when firstQuery is true. And right before we check the query, we set the self.firstQuery to false. So the next time query is triggered for any reason, the cache won't be touched. What do you think?
| success_list.append(to_use_list.pop(0)) | ||
| local_bad_backends=[] | ||
| for query_backend_curr in to_use_list: # query each of the backend list till found | ||
| specified_language = get_variable("tq_language", ['en']) |
There was a problem hiding this comment.
hmm, originally, I get the specified_language here because I would like to accomodate the case where the user change preferred language in different queries.
If specified language is obtained only when the plugin is initiated, then it won't reflect the later customizations made by the user until program restart.
| [state, synonym_list]=self.query_backends[query_backend_curr].query(word) | ||
| query_result = query_backend.query(word) | ||
| if (len(query_result) >= 3): | ||
| [state, synonym_list, antonym_list] = query_result |
There was a problem hiding this comment.
I understand that in most of the cases, so long as the word exists, it should have both synonym and antonym. But could some word list based engine contain some word's antonym but not synonym and vise versa? Do you think it's more prudent to add following logic:
if state==0:
if query_type==0 and no synonym_list:
state=1
if query_type==1 and no antonym_list:
state=1
| else: | ||
| [state, synonym_list] = query_result | ||
| antonym_list = [] | ||
| if query_type == 1: |
There was a problem hiding this comment.
this condition should be contained within a larger clause:
if state!=-1:
Or else it will fail to mark non-functional backend as bad backend, as it will fake a normal but empty state.
| continue | ||
| if state == 0: | ||
| # Update caches | ||
| update_cache(self.antonym_cache, word, antonym_list, query_backend.identifier) |
There was a problem hiding this comment.
since our state determination is based on the type of related word we try to search. Shouldn't our cache saving also based on our current query? Or else if the current search is on synonym from a backend that does not support antonym, then it will generate an empty cache for the antonym here.
Let's do
if state==0:
if query_type==0:
update_cache(self.antonym_cache, word, antonym_list, query_backend.identifier)
else:
update_cache(self.synonym_cache, word, synonym_list, query_backend.identifier)
| return self.last_valid_synonyms if query_type == 0 else self.last_valid_antonyms | ||
| else: | ||
| self.last_valid_result=synonym_list | ||
| self.last_valid_synonyms=synonym_list |
There was a problem hiding this comment.
let's only fill in one of the list with valid result at a time.
if query_type==0:
self.last_valid_synonyms=synonym_list
else
self.last_valid_antonyms=antonym_list
2 participants
Note: I recommend squashing the commits. I was playing around a lot with different devices, and so there's a ton of "work in progress" commits.