-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: MetaFieldRanker update #6742
Conversation
d160239
to
7fcef14
Compare
Pull Request Test Coverage Report for Build 7534119821Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
…ments are missing keys.
10d415a
to
0488e59
Compare
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.
Looks very good to me!
I left only a small comment to improve the expressiveness of the release note.
releasenotes/notes/metafieldranker_sort-order_refactor-2000d89dc40dc15a.yaml
Outdated
Show resolved
Hide resolved
…dc40dc15a.yaml Co-authored-by: Stefano Fiorucci <[email protected]>
Related Issues
infer_type
boolean discussed with @masci that would toggle whether we try to auto detect the type of the specified meta_field if all values of that meta field value are strings. Instead in this PR we focused on robustness of the MetaFieldRanker to fail less often by handling more edge cases and writing appropriate warning messages for them.Proposed Changes:
sort_order
that can have values ofdescending
orascending
. This allows the user to decide which direction to sort the meta values. Before we haddescending
hard-coded.ascending
could be useful, for example, ranking golfers were the the lowest score is best.How did you test it?
Notes for the reviewer
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.