lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Smiley (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SOLR-6797) Add score=degrees|kilometers|miles for AbstractSpatialFieldType
Date Tue, 30 Dec 2014 06:50:13 GMT

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

David Smiley commented on SOLR-6797:
------------------------------------

You're sort of on the right track with the exception of getting what I mean by a Units (or
perhaps better named DistanceUnits) class.  I think it's fine for it to be on the Solr side
(not Spatial4j).  There are several places where you're testing distanceUnits against constants,
which is a code smell.  Instead you could have an instance of DistanceUnits.  The back-compat
aspect means there's more to it but it will help.  An enum is tempting but someone might want
to add their own and we should let them... though there would need to be a hook to parse the
string value so some other one could be parsed (maybe simply a protected method on the field
type).

In AbstractSpatialFieldType.getSphereRadius, you were trying to come up with the sphere radius
for degrees.  That's solving for the radius of this equation: 2*pi*R = 360, which is 180 /
pi

The only other thing I noticed was that I'm not sure I like SpatialOptions.radius being <
0 as a means of saying it's unset.  Did you introduce this or was it -1 before but I didn't
notice?  I suggest a Double type and let it be null.

Thanks for pushing forward with this one, Ishan.
p.s. if you like, feel free to fork on GitHub and we can do code reviews there if convenient
for you. As the reviewer I found it *way* better than reviewing diffs to review progress for
my GSOC student, and I noticed your patch was a git diff.  Alternatively there's reviews.apache.org
but it didn't like your git diff file. IntelliJ had no issue with it on my svn checkout.

> Add score=degrees|kilometers|miles for AbstractSpatialFieldType
> ---------------------------------------------------------------
>
>                 Key: SOLR-6797
>                 URL: https://issues.apache.org/jira/browse/SOLR-6797
>             Project: Solr
>          Issue Type: Improvement
>          Components: spatial
>            Reporter: David Smiley
>         Attachments: SOLR-6797.patch, SOLR-6797.patch, SOLR-6797.patch, SOLR-6797.patch,
SOLR-6797.patch
>
>
> Annoyingly, the units="degrees" attribute is required for fields extending AbstractSpatialFieldType
(e.g. RPT, BBox).  And it doesn't really have any effect.  I propose the following:
> * Simply drop the attribute; ignore it if someone sets it to "degrees" (for back-compat).
> * When using score="distance", or score=area or area2D (as seen in BBoxField) then use
kilometers if geo=true, otherwise degrees.
> * Add support for score=degrees|kilometers|miles|degrees



--
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


Mime
View raw message