[jira] [Commented] (LUCENE-6997) Graduate GeoUtils and postings based GeoPointField from sandbox...

["Steve Rowe (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LUCENE-6997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15132364#comment-15132364 ] 

Steve Rowe commented on LUCENE-6997:
------------------------------------

Hi Nick, I skimmed the patch and noticed a few things:

* You have some non-related whitespace changes in various files, very likely introduced by your IDE.  Please change the appropriate setting so that it only auto-trims whitespace on changed lines.
* In {{dev-tools/idea/lucene/spatial/spatial.iml}}, you removed {{scope="TEST"}} from the {{lucene-test-framework}} module dependency - why?:
\\
{noformat}
-    <orderEntry type="module" scope="TEST" module-name="lucene-test-framework" />
[...]
+    <orderEntry type="module" module-name="lucene-test-framework" />
{noformat}
* AFAICT, the benchmark module no longer depends on the spatial module, so I think you can remove the {{jar-spatial}} target from the {{init}} target's list of dependencies In {{lucene/benchmark/build.xml}}?:
\\
{noformat}
-    <target name="init" depends="module-build.init,jar-memory,jar-highlighter,jar-analyzers-common,jar-queryparser,jar-facet,jar-spatial,jar-codecs,jar-join"/>
+    <target name="init" depends="module-build.init,jar-memory,jar-highlighter,jar-analyzers-common,jar-queryparser,jar-facet,jar-spatial,jar-spatial-extras,jar-codecs,jar-join"/>
{noformat}
* The {{spatial-compile-test}} target you added to {{lucene/module-build.xml}} is only used in the benchmark module, so maybe it could move there?  That's the strategy used by the build for the dataimporthandler-extras module, which (alone) depends on the dataimporthandler module's tests.  A nit: most other module-test-building targets (see {{solr/common-build.xml}}) are named {{compile-test-<module>}}, not {{<module>-compile-test}}:
\\
{noformat}
+  <target name="spatial-compile-test" depends="init" if="module.has.tests">
+    <ant dir="${common.dir}/spatial" target="compile-test" inheritAll="false"/>
+  </target>
{noformat}
* Other than adding a POM template for the spatial-extras module, and including it in the Lucene parent POM's list of sub-modules to build, you didn't modify any inter-module dependencies in other POM templates, and I'm pretty sure there are several changes to make (looking at the Ant and IntelliJ build changes you made).

Later today I'll try to run the IntelliJ and Maven builds.

> Graduate GeoUtils and postings based GeoPointField from sandbox...
> ------------------------------------------------------------------
>
>                 Key: LUCENE-6997
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6997
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: modules/spatial
>            Reporter: Nicholas Knize
>         Attachments: LUCENE-6997.patch
>
>
> {{GeoPointField}} is a lightweight dependency-free postings based geo field currently in sandbox. It has evolved into a very fast lightweight geo option that heavily leverages the optimized performance of the postings structure. It was originally intended to graduate to core but this does not seem appropriate given the variety of "built on postings" term encoding options (e.g., see LUCENE-6930).  
> Additionally, the {{Geo*Utils}} classes are dependency free lightweight relational approximation utilities used by both {{GeoPointField}} and the BKD based {{LatLonField}} and can also be applied to benefit the lucene-spatial module.
> These classes have been evolving and baking for some time and are at a maturity level qualifying for promotion from sandbox. This will allow support for experimental encoding methods with (minimal) backwards compatibility - something sandbox does not allow.
> Since GeoPoint classes are dependency free, all GeoPointField and support and utility classes currently in sandbox would be promoted to the spatial3d package. (possibly a separate issue to rename spatial3d to spatialcore or spatiallite?) Such that for basic lightweight Geo support one would only need a handful of lucene jars. By simply adding the lucene-spatial module and its dependency jars users can obtain more advanced geospatial support (heatmap facets, full shape relations, etc).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org