You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by osma <gi...@git.apache.org> on 2015/05/25 15:32:59 UTC

[GitHub] jena pull request: Add (?uri ?score) to jena-text

GitHub user osma opened a pull request:

    https://github.com/apache/jena/pull/72

    Add (?uri ?score) to jena-text

    This is an implementation of [JENA-916](https://issues.apache.org/jira/browse/JENA-916), adding support for using a 2-element list as the subject of a text:query triple pattern to capture the raw Lucene/Solr score of the result, i.e. (?s ?score) text:query "word" .
    
    Some notes:
    
    1. This changes the return type of search-related methods in TextIndex and DatasetGraphText to List<TextHit>, when it used to be List<Node>. (The new class TextHit encapsulates both the Node and the score.) I've changed all the related code and tests to cope with this, but there may be external code that relies on the old return type. Is this a problem? Do we need to leave the old method signatures for compatibility (possibly @Deprecated) and introduce new score-aware methods?
    
    2. I tried to adjust the Solr side as well but I'm not sure that the jena-text Solr backend even works at the moment (even the unit tests are disabled). I couldn't get the current code (prior to this patch) to work with Solr - the problem is the lack of a unique identifier field that Solr seems to require. But I'm not a Solr expert and perhaps it can be configured to work with jena-text. As it is, I cannot test that the score capturing actually works with Solr.
    
    3. I took some inspiration from LARQ and jena-csv. I can't say I have a deep understanding of QueryIterators, Bindings and BindingMaps though and I hope I didn't end up cargo-culting any bad practices from older code...

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/osma/jena jena-text-score

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/jena/pull/72.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #72
    
----
commit 4c99ec53510fc9bdda1f66a42636b3185e071e98
Author: Osma Suominen <os...@aalto.fi>
Date:   2015-05-25T12:54:56Z

    First implementation of JENA-916: Add (?uri ?score) to text query.

commit 3f0c5173eb59b37011d985e491a73cdd2dc93b31
Author: Osma Suominen <os...@aalto.fi>
Date:   2015-05-25T12:57:38Z

    Merge remote-tracking branch 'upstream/master'

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107149537
  
    There are a few `// *** score` markers left - presumably these can all go now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/72#discussion_r31388520
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextHitConverter.java ---
    @@ -0,0 +1,50 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.jena.query.text ;
    +
    +import java.util.function.Function;
    +
    +import org.apache.jena.sparql.core.Var;
    +import org.apache.jena.sparql.engine.binding.Binding;
    +import org.apache.jena.sparql.engine.binding.BindingFactory;
    +import org.apache.jena.sparql.engine.binding.BindingMap;
    +import org.apache.jena.sparql.util.NodeFactoryExtra;
    +
    +/** Class that converts TextHits to Bindings that can be returned from query */ 
    +public class TextHitConverter implements Function<TextHit, Binding>
    +{
    +    private Binding binding;
    +    private Var match;
    +    private Var score;
    +
    +    public TextHitConverter(Binding binding, Var match, Var score) {
    +        this.binding = binding;
    +        this.match = match;
    +        this.score = score;
    +    }
    +    
    --- End diff --
    
    Need `@Override`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-106892384
  
    > 1. I dont think that change is a problem and \@Deprecated isn't needed
    Great, no worries then!
    
    > 1. IIRC solr works differently and puts in a "score" field in the results.
    Yes, I looked at some Solr example code which handled scores and tried to follow that. But as I said I was unable to hook up Solr to current jena-text even without my changes, so I wasn't able to test my code for real.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-106861440
  
    @osma - I'll find some time to review this but 
    
    To your points:
    1. I dont think that change is a problem and  \@Deprecated isn't needed because (a) Jena3 allows us to be a bit more flexibility to do things right and (b) the major use is via SPARQL and that is not affected.  I don't recall anyone asking about detailed direct use of the module.
    1. IIRC solr works differently and puts in a "score" field in the results.
    1. Yes - we already have enough old practices! I'll look at the PR for these. 
    
    I'd like to proceed getting functionality in and worry about clearing up very soon after. It exposes the changes early for those interested.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/72#discussion_r31388541
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -165,37 +176,48 @@ public QueryIterator exec(Binding binding, PropFuncArg argSubject, Node predicat
             // ----
     
             QueryIterator qIter = (Var.isVar(s)) 
    -            ? variableSubject(binding, s, match, execCxt) 
    -            : concreteSubject(binding, s, match, execCxt) ;
    +            ? variableSubject(binding, s, score, match, execCxt) 
    +            : concreteSubject(binding, s, score, match, execCxt) ;
             if (match.getLimit() >= 0)
                 qIter = new QueryIterSlice(qIter, 0, match.getLimit(), execCxt) ;
             return qIter ;
         }
     
    -    private QueryIterator variableSubject(Binding binding, Node s, StrMatch match, ExecutionContext execCxt) {
    -        Var v = Var.alloc(s) ;
    -        List<Node> r = query(match.getQueryString(), match.getLimit(), execCxt) ;
    -        // Make distinct. Note interaction with limit is imperfect
    -        r = Iter.iter(r).distinct().toList() ;
    -        QueryIterator qIter = new QueryIterExtendByVar(binding, v, r.iterator(), execCxt) ;
    +    private QueryIterator variableSubject(Binding binding, Node s, Node score, StrMatch match, ExecutionContext execCxt) {
    +        Var sVar = Var.alloc(s) ;
    +        Var scoreVar = (score==null) ? null : Var.alloc(score) ;
    +        List<TextHit> r = query(match.getQueryString(), match.getLimit(), execCxt) ;
    +        Function<TextHit,Binding> converter = new TextHitConverter(binding, sVar, scoreVar);
    +        Iterator<Binding> bIter = new Map1Iterator<TextHit, Binding>(converter, r.iterator());
    --- End diff --
    
    There is a library of iterator utilities:
    ```
    Iterator<Binding> bIter = Iter.map(r.iterator(), converter) ;
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/jena/pull/72


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107946460
  
    Thank you for merging! What should be done with the documentation?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107835602
  
    I have addressed Andy's point-wise comments in separate commits. The only remaining issue is whether to make the API change for `search` methods. 
    
    After some research, I'm strongly in favour of changing the API in Jena3, as is done in this PR. I see no point in retaining the old methods, which I suspect approximately nobody uses. Alex said he's using SPARQL queries (which are unaffected), and I also checked epimorphics' [ppd-text-index](https://github.com/epimorphics/ppd-text-index) which is an extension of jena-text but seems to be unaffected by the API change as it's not calling any of the affected `search` methods.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by amiara514 <gi...@git.apache.org>.
Github user amiara514 commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107581689
  
    >Maybe @amiara514 would have a comment as I understood he is using jena-text via Java code? 
    
    @osma : Not exactly, I execute Sparql queries via java code which involve jena-text. I don't manipulate it at this level.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/72#discussion_r31388531
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -27,11 +29,13 @@
     import org.apache.jena.datatypes.xsd.XSDDatatype ;
     import org.apache.jena.graph.Node ;
     import org.apache.jena.query.QueryBuildException ;
    +import org.apache.jena.query.QueryExecException ;
     import org.apache.jena.sparql.core.* ;
     import org.apache.jena.sparql.engine.ExecutionContext ;
     import org.apache.jena.sparql.engine.QueryIterator ;
     import org.apache.jena.sparql.engine.binding.Binding ;
     import org.apache.jena.sparql.engine.iterator.QueryIterExtendByVar ;
    --- End diff --
    
    `QueryIterExtendByVar` : Unused import


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107519784
  
    `Iter` is also a good choice because it provides access to transform-behavior as well as many other forms of iterator-processing. `Map1Iterator` is more of an "implementation" class. As the code evolves, I would expect `Iter` to last longer and be better supported.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107944800
  
    Merge done! Thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/72#discussion_r31388532
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -18,7 +18,9 @@
     
     package org.apache.jena.query.text ;
     
    +import java.util.Iterator ;
     import java.util.List ;
    +import java.util.function.Function ;
     
     import org.apache.jena.atlas.iterator.Iter ;
     import org.apache.jena.atlas.lib.InternalErrorException ;
    --- End diff --
    
    `InternalErrorException` : Unused import - should be removed. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107516262
  
    Re: Map1Iterator -- yes is it similar (and older).  `Iter` works on any iterators, not just `ExtendedIterator`.
    
    Re: naming of search - your choice. It is alwayss difficult to choose between compatibility and creating legacy.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107341239
  
    Thanks a lot Andy for your review! I will address your points soon and push a new version.
    
    Thank you also for pointing out Iter.map(), I actually wanted something like that when coding but then I found Map1Iterator (which had been recently upgraded to support Java 8 Functions) and settled on using that.
    
    >I'm unsure whether it is better to add searchWithScore (or better name?), which returns the List<TextHit> form, and retain search returning List<Node> (just a Iter.map added each time):
    
    I wouldn't call it searchWithScore - potentially, TextHit could also contain other information than the node and score. searchWithHits? searchForHits? searchTextHits?
    
    I only use jena-text via SPARQL so I can't really tell whether the API breakage would be bad. Maybe @amiara514 would have a comment as I understood he is using jena-text via Java code? My opinion is slightly on the "clean up the API with Jena3, don't worry about breakage" side.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/72#discussion_r31388513
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextHitConverter.java ---
    @@ -0,0 +1,50 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.jena.query.text ;
    +
    +import java.util.function.Function;
    +
    +import org.apache.jena.sparql.core.Var;
    +import org.apache.jena.sparql.engine.binding.Binding;
    +import org.apache.jena.sparql.engine.binding.BindingFactory;
    +import org.apache.jena.sparql.engine.binding.BindingMap;
    +import org.apache.jena.sparql.util.NodeFactoryExtra;
    +
    +/** Class that converts TextHits to Bindings that can be returned from query */ 
    +public class TextHitConverter implements Function<TextHit, Binding>
    +{
    +    private Binding binding;
    +    private Var match;
    +    private Var score;
    +
    +    public TextHitConverter(Binding binding, Var match, Var score) {
    +        this.binding = binding;
    +        this.match = match;
    +        this.score = score;
    +    }
    +    
    +    public Binding apply(TextHit hit) {
    +        BindingMap bmap = BindingFactory.create(binding);
    +        bmap.add(match, hit.getNode());
    +        if (score != null) {
    +            bmap.add(score, NodeFactoryExtra.floatToNode(hit.getScore()));
    +        }
    +        return bmap;
    +    }
    --- End diff --
    
    Minor improvement: `BindingMap` is a map so it's bigger than the one slot binding. This is slightly better when not returning the score:
    ```
        public Binding apply(TextHit hit) {
            if (score == null) 
                return BindingFactory.binding(binding, match, hit.getNode()) ;
            BindingMap bmap = BindingFactory.create(binding);
            bmap.add(match, hit.getNode());
            bmap.add(score, NodeFactoryExtra.floatToNode(hit.getScore()));
            return bmap ;
        }
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107954622
  
    I decided to not bother with CMS but attached a diff into JIRA, it is available at https://issues.apache.org/jira/secure/attachment/12736839/jena-text-score-doc.diff


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107947521
  
    An update for http://jena.staging.apache.org/documentation/query/text-query.html would be good.  Either via through the CMS UI  or a diff to https://svn.apache.org/repos/asf/jena/site/trunk/content/documentation/query/text-query.mdtext.  Or if those are problematic, text changes emailed to dev@ as I assume we are not talking about a vast change.
    
    (caution - as @amiara514 experienced, "[update]" then "update this resource" may be necessary)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/72#discussion_r31388517
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextHitConverter.java ---
    @@ -0,0 +1,50 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.jena.query.text ;
    +
    +import java.util.function.Function;
    +
    +import org.apache.jena.sparql.core.Var;
    +import org.apache.jena.sparql.engine.binding.Binding;
    +import org.apache.jena.sparql.engine.binding.BindingFactory;
    +import org.apache.jena.sparql.engine.binding.BindingMap;
    +import org.apache.jena.sparql.util.NodeFactoryExtra;
    +
    +/** Class that converts TextHits to Bindings that can be returned from query */ 
    +public class TextHitConverter implements Function<TextHit, Binding>
    +{
    +    private Binding binding;
    +    private Var match;
    +    private Var score;
    +
    +    public TextHitConverter(Binding binding, Var match, Var score) {
    +        this.binding = binding;
    +        this.match = match;
    +        this.score = score;
    +    }
    +    
    +    public Binding apply(TextHit hit) {
    --- End diff --
    
    Need `@Override`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-106794036
  
    I updated my branch to fix merge conflicts caused by recent jena-text commits. Would this be ready for merging? @afs ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107150120
  
    About changing return result from `DatasetGraphText.search`:
    
    I'm unsure whether it is better to add `searchWithScore` (or better name?), which returns the `List<TextHit>` form, and retain `search` returning `List<Node>` (just a `Iter.map` added each time):
    
    Example:
    ```
        /** Search the text index on the default text field */
        public Iterator<TextHit> searchWithScore(String queryString) {
            return search(queryString, null) ;
        }
    
        /** Search the text index on the default text field */
        public Iterator<Node> search(String queryString) {
            return Iter.map(searchWithScore(queryString, textHit->textHit.getNode()) ;
        }
    ```
    If the change rote, then Jena3 is the time to do it.
    
    What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: Add (?uri ?score) to jena-text

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:

    https://github.com/apache/jena/pull/72#issuecomment-107150565
  
    Other than the point-wise comments above, all looks good and the tests pass for me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---