Implement getScores() and getMaxScore() for EDS results #4807
Conversation
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @maccabeelevine -- see below for some more food for thought.
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
@maccabeelevine, thanks for the improvements. I think this looks pretty good -- but see below for one nagging concern. If that's not really an issue, we can probably merge this, but I wanted to discuss first just in case! :-)
| if (empty($this->results)) { | ||
| return null; | ||
| } | ||
| return $this->results[0]->getScore(); |
There was a problem hiding this comment.
Presumably this only works if we have a relevance-descending sort. Maybe that's the only time that we need it to work. But are there other cases we need to worry about?
There was a problem hiding this comment.
Good catch. In my use case (#4806) I would probably only care in the context of a relevancy sort, but other uses might care about other sorts. The behavior I'm seeing from the EDS API is that it doesn't actually calculate a real relevancy score if you have any other sort, however it returns zero for every result instead of just omitting the score. For the purposes of our API, it's better to return null in that situation.
| { | ||
| if ( | ||
| empty($this->results) || | ||
| 'relevance' != $this->getParams()->getSort() |
There was a problem hiding this comment.
This theoretically leaves open the absurd case of sorting by least-relevant first, but looking at the way EDS does sort key naming I think that would actually have a different name, something like "relevance2". If it is even supported by the API, which (hopefully) it's not. All the API docs tell you is that you can see available sort options from the /info endpoint, and so for my instance there's no reverse-sort option, but it's possible that I could turn it on somewhere in EBSCOadmin if I dared to look.
There was a problem hiding this comment.
I'm willing to live with the assumption that we won't have to deal with this scenario.
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Looks good to me now. Thanks, @maccabeelevine!
2 participants
These functions are stub defined in Search/Base/Results, and overridden only for Solr. Implement them for EDS Results.