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

Improving the scope of search results #71

Closed
wants to merge 10 commits into from

Conversation

sarthikadhawan
Copy link

@sarthikadhawan sarthikadhawan commented Feb 27, 2018

Checklist

  • [x ] My branch is up-to-date with the upstream develop branch.
  • I have added necessary documentation (if appropriate). [Not applicable]

Which issue does this PR fix?:
fixes: #65 #70

This PR resolves issue #70 (The random keywords in search box aren't always having a search result). Following is obtained upon typing "Logistic Regression" onto the searchbox.

screencapture-localhost-8000-query-1519736762577

The above search displays the correct results.
However, following is obtained upon typing "logistic regression" (which is basically one of the randomly generated keywords which show up upon launching Cosmos Search).

screencapture-search-opengenus-org-query-1519736808345

Finally, following is obtained upon typing "logistic regression" after widening the search scope a bit.

screencapture-localhost-8000-query-1519736859544

This PR also resolves the pluralization issue.

Why do we need this PR?:

Most users expect the same results while searching for "Logistic Regression", "logistic regression" etc. The search should be able to handle such cases. We aim to broaden the search experience of users through this PR and the PRs that follow.

@AdiChat Please review.

Thanks,
Team GalSquared | RGSoC

@sakshee-19
Copy link
Member

@xx621998xx I have already sent PR for the issue #65.

@sarthikadhawan
Copy link
Author

@sakshee-19 That's cool!
This PR majorly focuses on query optimization (#70) as discussed, so no issues (pun intended). :D

@AdiChat
Copy link
Member

AdiChat commented Mar 1, 2018

Nice work 👍

The issue (#70) is related to keywords like hierachical clustering which is not being considered by the current implementation. Note that, we get results for hierachical-clustering.

You have capitalized the first letters but this could be avoided by doing a matching after converting both data to lowercase. Instead, you must generate permutation in terms of order of words and separate them with _ and -.

Note that linear search returns a result but search linear does not.
Thus, for keyword linear search, the permutations generated must be linear_search, linear-search, search_linear and search-linear.

Kindly make appropriate changes and let us know when it is ready for review. 👍

@soc221b
Copy link

soc221b commented Mar 1, 2018

@AdiChat I think we should always replace - with _ to reduce complexity of search strategy.
In this way, we can just change metadata generator in update/view.py.

If you agree, I will implement this.

@sarthikadhawan
Copy link
Author

@iattempt , we have already started working on the issue :)

search/views.py Outdated
perms.append(word[0].upper() + word[1:] )
count = count+1
p='_'.join(perms)
final.append(p)
Copy link

Choose a reason for hiding this comment

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

In my opinion, we should sort combinations by length and/or else.
In general, one want get the most matched result with the tags she/he typed.

@AdiChat
Copy link
Member

AdiChat commented Mar 2, 2018

@iattempt Sure. I guess you are suggesting to remove the inconsistency in Cosmos structure and update the metadata generator. This will be great. 👍

@xx621998xx You can continue working on this issue, assuming, - has been replaced by _.
Thus, for keyword linear search, the permutations generated must be linear_search, search_linear, linear and search. The final result should be the one having the minimum positive number of results. 👍

@soc221b
Copy link

soc221b commented Mar 5, 2018

I think we should extract/refactor class by the function query in search/views.

Could I work on this?

@AdiChat
Copy link
Member

AdiChat commented Mar 6, 2018

@iattempt Sure, Go for it 👍

@sarthikadhawan
Copy link
Author

@AdiChat Kindly review the changes.

@AdiChat
Copy link
Member

AdiChat commented Mar 14, 2018

You need to clean the files associated with this pull request. There are 119 files. Kindly take a look. 👍

@sarthikadhawan
Copy link
Author

Hi
We are opening another pull request for the same.

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.

Pluralization Bug When Searching for Query with Only One Result
4 participants