[
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: [email protected]
For additional commands, e-mail: [email protected]
