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
function_score: undo "Remove explanation of query score from functions" #9826
Conversation
This adds the Explanation to the explain score again. It is needed because the explanation of script functions will otherwise not contain an explanation of _score if boost mode is set to replace.
return new Explanation((float) (runAsDouble()), "This script returned " + runAsDouble()); | ||
public Explanation explain(Explanation subQueryScore) throws IOException { | ||
Explanation exp = new Explanation((float) (runAsDouble()), "This script returned " + runAsDouble()); | ||
Explanation _scoreExp = new Explanation(subQueryScore.getValue(), "_score: "); |
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.
Can you remove the leading underscore? Variables should start with a lowercase letter according to http://www.oracle.com/technetwork/java/codeconventions-135099.html
LGTM |
@@ -86,6 +88,7 @@ public void testNativeExplainScript() throws InterruptedException, IOException, | |||
for (SearchHit hit : hits.getHits()) { | |||
assertThat(hit.getId(), equalTo(Integer.toString(idCounter))); | |||
assertThat(hit.explanation().toString(), containsString(Double.toString(idCounter) + " = This script returned " + Double.toString(idCounter))); | |||
assertThat(hit.explanation().toString(), containsString("1.0 = tf(freq=1.0), with freq of")); |
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.
To prevent future changes from removing the nesting, we could add for example
assertThat(hit.explanation().getDetails().length, equalTo(2));
or something even more explicit.
@brwe WDYT?
👍 |
Can we expect this to be in version 1.4.5? |
@brwe LGTM |
@brwe please merge this into 1.x, 1.5 and master |
This adds the Explanation to the explain score again. It is needed because the explanation of script functions will otherwise not contain an explanation of _score if boost mode is set to replace. closes #9826
This adds the Explanation to the explain score again. It is needed because the explanation of script functions will otherwise not contain an explanation of _score if boost mode is set to replace. closes #9826
@maxjakob sorry we did not get this in 1.5 and promised I'll add labels next time to remind me. It will be in 1.5.1. There will be no further 1.4.X release. |
@brwe I see, 1.5.1 then, that's fine. Thanks! |
This adds the Explanation to the explain score again. It is needed because the explanation of script functions will otherwise not contain an explanation of _score if boost mode is set to replace. closes elastic#9826
This adds the Explanation to the explainScore() again. It is needed
because the explanation of script functions will otherwise not contain
an explanation of _score if boost mode is set to replace.