[
https://issues.apache.org/jira/browse/LUCENE-7951?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16152635#comment-16152635
]
David Smiley commented on LUCENE-7951:
--------------------------------------
Thanks for contributing this Ignacio!
Geo3dSpatialContextFactory:
* this should override init() in order to populate the planet model. See the
SCF impl to see how this is done for other settings. On the Solr side, the
field type settings will be read using init().
* make planetModel field public and not final. This is to be consistent with
SCF. I know it's a bit unusual.
* override initCalculator() to see if the value is something like "geo3d" and
if so then set distCalc that way, let the superclass handle it.
I was a little surprised to see you didn't subclass SpatialContext with a Geo3d
one but it's cool that you didn't have to, actually.
Geo3dShapeFactory:
* Constructor: Should not reference {{PlanetModel.WGS84}} explicitly but
instead reference a proposed Geo3dSpatialContextFactory.DEFAULT_PLANET_MODEL ?
Or instead, just assume Geo3dSpatialContextFactory is in use and fail with the
ClassCastException if it isn't? I like the simplicity of the latter; either
work for me.
* Should probably have the "normWrapLongitude" logic that can be seen in
ShapeFactoryImpl. Likewise the lat/lon bounds verification. You can probably
have the verify logic assume decimal degrees here (-/+ 180 etc.) since Geo3d
assumes this.
* inner classes: the default constructors found in this class are superfluous;
they can be removed. Also these classes can be made private.
Geo3dPointShape:
* consider removing "Shape" suffix; the Circle, Rectangle ones don't have this.
Or maybe they should have them? I don't care; just be consistent.
* minor: typo in javadoc for Geo3dAreShape. I prefer to use {{{@link ... }}}
in general as the IDE helps keep this right.
Geo3dDistanceCalculator:
* It appears this is only necessary when the planet model isn't a sphere, as
Spatial4j's default dist calculator is for spherical. I suggest letting the
distance calculator default. I worry that Geo3d's impl is too slow; it sure
looks slow!
I'll review more of this code later.
> New wrapper classes for Geo3d
> -----------------------------
>
> Key: LUCENE-7951
> URL: https://issues.apache.org/jira/browse/LUCENE-7951
> Project: Lucene - Core
> Issue Type: Improvement
> Components: modules/spatial-extras
> Reporter: Ignacio Vera
> Priority: Minor
> Attachments: LUCENE-7951.patch
>
>
> Hi,
> After the latest developments in the Geo3d library, in particular:
> [https://issues.apache.org/jira/browse/LUCENE-7906] : Spatial relationships
> between GeoShapes
> [https://issues.apache.org/jira/browse/LUCENE-7936]: Serialization of
> GeoShapes.
> I propose a new set of wrapper classes which can be for example linked to
> Solr as they implement their own SpatialContextFactory. It provides the
> capability of indexing shapes with
> spherical geometry.
> Thanks!
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
