hidingmyname opened a new issue, #16921:
URL: https://github.com/apache/lucene/issues/16921
### Summary
Both `idfExplain` overloads in `BM25Similarity` have an `@return` that says
"an Explain object". There's no `Explain` type. The methods return
`org.apache.lucene.search.Explanation`. `TFIDFSimilarity` has the same two
overloads with the same wording, so I've included it here too.
### Where
`lucene/core/src/java/org/apache/lucene/search/similarities/BM25Similarity.java:172-198`:
```java
* @param fieldStats field-level statistics
* @param termStats term-level statistics for the term
* @return an Explain object that includes both an idf score factor and an
explanation for the
* term.
*/
public Explanation idfExplain(FieldStats fieldStats, TermStats termStats) {
```
```java
* @param fieldStats field-level statistics
* @param termStats term-level statistics for the terms in the phrase
* @return an Explain object that includes both an idf score factor for
the phrase and an
* explanation for each term.
*/
public Explanation idfExplain(FieldStats fieldStats, TermStats[]
termStats) {
```
`lucene/core/src/java/org/apache/lucene/search/similarities/TFIDFSimilarity.java:368-392`
has the same text:
```java
* @return an Explain object that includes both an idf score factor and an
explanation for the
* term.
*/
public Explanation idfExplain(FieldStats fieldStats, TermStats termStats) {
...
* @return an Explain object that includes both an idf score factor for
the phrase and an
* explanation for each term.
*/
public Explanation idfExplain(FieldStats fieldStats, TermStats[]
termStats) {
```
Those four lines are the only matches for "Explain object" on `main`.
### Why it matters
These are public methods meant to be overridden. Their javadoc is where you
learn how to customize idf in BM25 or classic TF-IDF scoring, so people
subclassing a similarity read it closely. The return type is also the hook
for
`IndexSearcher.explain()` output, so a reader who searches for an `Explain`
class to see what to build won't find one. It's a small fix, but it's in
core's public scoring API.
### Suggested fix
In `BM25Similarity.java` and in `TFIDFSimilarity.java`:
```diff
- * @return an Explain object that includes both an idf score factor and
an explanation for the
- * term.
+ * @return an {@link Explanation} that includes both an idf score factor
and an explanation for
+ * the term.
```
```diff
- * @return an Explain object that includes both an idf score factor for
the phrase and an
- * explanation for each term.
+ * @return an {@link Explanation} that includes both an idf score factor
for the phrase and an
+ * explanation for each term.
```
(`Explanation` is already imported in both files.)
### Upstream reference
Lucene.NET fixed the BM25 copy in its port in apache/lucenenet@`2a1541c18`
("Lucene.Net.Search.Similarities: Fixed up documentation comments"):
```diff
--- a/src/Lucene.Net/Search/Similarities/BM25Similarity.cs
+++ b/src/Lucene.Net/Search/Similarities/BM25Similarity.cs
/// <param name="collectionStats"> collection-level statistics
</param>
/// <param name="termStats"> term-level statistics for the term
</param>
- /// <returns> an Explain object that includes both an idf score
factor
+ /// <returns> an <see cref="Explanation"/> object that includes
both an idf score factor
/// and an explanation for the term. </returns>
public virtual Explanation IdfExplain(CollectionStatistics
collectionStats, TermStatistics termStats)
{
@@ -197,13 +198,13 @@ public virtual Explanation
IdfExplain(CollectionStatistics collectionStats, Term
...
/// <param name="collectionStats"> collection-level statistics
</param>
/// <param name="termStats"> term-level statistics for the terms in
the phrase </param>
- /// <returns> an Explain object that includes both an idf
+ /// <returns> an <see cref="Explanation"/> object that includes
both an idf
/// score factor for the phrase and an explanation
/// for each term. </returns>
public virtual Explanation IdfExplain(CollectionStatistics
collectionStats, TermStatistics[] termStats)
```
Current Lucene.NET `BM25Similarity.cs`:
```csharp
/// <returns> an <see cref="Explanation"/> object that includes both
an idf score factor
...
/// <returns> an <see cref="Explanation"/> object that includes both
an idf
```
Lucene.NET did not fix the `TFIDFSimilarity` copy. Its `TFIDFSimilarity.cs`
still says "An Explain object" at both overloads, so the TF-IDF half of this
issue is my extension, not something taken from the port. Since it's the
same sentence on the same method, it seemed better to fix both in one change.
I can submit a PR for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
