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

Merriam Webster Backend, Antonyms, and Result Caching #48

Open
wants to merge 42 commits into
base: testing
Choose a base branch
from

Conversation

ahayman
Copy link

@ahayman ahayman commented Oct 2, 2020

  • Added the Merriam-Webster Backend
  • Added support for antonym queries (currently only supported by Merriam-Webster Backend)
  • Added Caching Logic

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.

Aaron Hayman added 30 commits September 16, 2020 11:08
Copy link
Owner

@Ron89 Ron89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

for query_backend_curr in to_use_list: # query each of the backend list till found
specified_language = get_variable("tq_language", ['en'])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

else:
self.last_valid_result=synonym_list
self.last_valid_synonyms=synonym_list
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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 this pull request may close these issues.

2 participants