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

Add Scorer option to Query #113

Merged
merged 7 commits into from
Mar 6, 2021
Merged

Add Scorer option to Query #113

merged 7 commits into from
Mar 6, 2021

Conversation

chrisdiana
Copy link
Contributor

@chrisdiana chrisdiana commented Feb 9, 2021

Add support for using custom scoring functions in a Redisearch query.

Closes #35

TFIDF is the default scoring function but this PR allows users to attach custom scoring functions to a Query object.

  • TFIDF.DOCNORM
  • BM25
  • DISMAX
  • DOCSCORE
  • HAMMING
res = client.search(Query('foo').scorer('TFIDF.DOCNORM'))

https://oss.redislabs.com/redisearch/Scoring/

@codecov
Copy link

codecov bot commented Feb 9, 2021

Codecov Report

Merging #113 (cbc87cb) into master (f3f11a4) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #113      +/-   ##
==========================================
+ Coverage   90.02%   90.17%   +0.14%     
==========================================
  Files          12       12              
  Lines        1675     1700      +25     
==========================================
+ Hits         1508     1533      +25     
  Misses        167      167              
Impacted Files Coverage Δ
redisearch/query.py 90.50% <100.00%> (+0.37%) ⬆️
test/test.py 96.01% <100.00%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3f11a4...cbc87cb. Read the comment docs.

@chrisdiana
Copy link
Contributor Author

@emmanuelkeller please let me know if there's anything you'd like me to add to this PR. thanks!

Copy link
Contributor

@ashtul ashtul left a comment

Choose a reason for hiding this comment

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

Hi @chrisdiana,
Thank you for the PR.
Can you extend the test to include all types of scorers? In addition, add withscore so we can see the value changes.
Thanks

@chrisdiana
Copy link
Contributor Author

Thanks @ashtul I've updated the PR with tests for all scorers with scores. Let me know if you want me to add anything else

@sonarcloud
Copy link

sonarcloud bot commented Mar 5, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@ashtul ashtul left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Thank you @chrisdiana

@ashtul ashtul merged commit 8122d94 into RediSearch:master Mar 6, 2021
Comment on lines +155 to +156
deps/*
Copy link
Contributor

Choose a reason for hiding this comment

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

@chrisdiana Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ashtul when you initially are setting up tests on a new repo and run make -C test/docker test PYTHON_VER=3 generated a ./deps directory that was dumped into my root path with "readies" inside it. It seemed like someething that wasn't cleaned up that maybe should have so I added to .gitignore in case the cleanup failed on others environments. I can remove it if you like though.

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.

does the client support TFIDF scoring and multisearch?
2 participants