[Xapian-tickets] [Xapian] #481: Merge geospatial branch

[Xapian]

#481: Merge geospatial branch
-------------------------+--------------------------------------------------
 Reporter:  richard      |       Owner:  richard  
     Type:  enhancement  |      Status:  new      
 Priority:  normal       |   Milestone:  1.2.1    
Component:  Library API  |     Version:  SVN trunk
 Severity:  normal       |    Keywords:           
Blockedby:               |    Platform:  All      
 Blocking:               |  
-------------------------+--------------------------------------------------

Old description:

> We've had a few requests for the features in the geospatial branch, so
> I've put together a clean patch against trunk of what I think are the
> stable and reasonably "uncontroversial" parts of the geospatial branch.
> This patch omits several things from the geospatial branch:
>
>  - The LatLongDistanceMatchDecider, since this is superceded by
> functionality in the LatLongDistancePostingSource, and we're moving
> towards deprecating MatchDeciders.
>
>  - The parser for latitude-longitude coordinates.  This is quite handy to
> allow lots of arbitrary formats for coordinates to be parsed into numeric
> values, but is a bit complex, and is a second lemon parser to be
> maintained, so I'd rather consider merging it at a later date.  Without
> the parser, coordinates have to be supplied as a pair of doubles, rather
> than having the option of providing them as strings.
>
>  - The half-completed HTM based generation of "acceleration" terms.
>
> I've tidied up the documentation and tests, and fixed a few small issues
> in the patch, so I think this is ready to be reviewed and considered for
> merging to trunk.  I'd be happy if it went in for 1.2.1, or at least got
> considered for it, so marking that as the milestone for now.

New description:

 We've had a few requests for the features in the geospatial branch, so
 I've put together a clean patch against trunk of what I think are the
 stable and reasonably "uncontroversial" parts of the geospatial branch.
 This patch omits several things from the geospatial branch:

  - The !LatLongDistanceMatchDecider, since this is superceded by
 functionality in the !LatLongDistancePostingSource, and we're moving
 towards deprecating !MatchDeciders.

  - The parser for latitude-longitude coordinates.  This is quite handy to
 allow lots of arbitrary formats for coordinates to be parsed into numeric
 values, but is a bit complex, and is a second lemon parser to be
 maintained, so I'd rather consider merging it at a later date.  Without
 the parser, coordinates have to be supplied as a pair of doubles, rather
 than having the option of providing them as strings.

  - The half-completed HTM based generation of "acceleration" terms.

 I've tidied up the documentation and tests, and fixed a few small issues
 in the patch, so I think this is ready to be reviewed and considered for
 merging to trunk.  I'd be happy if it went in for 1.2.1, or at least got
 considered for it, so marking that as the milestone for now.

--

Comment(by olly):

 Some minor code-style issues:

  * We're using C++ forms of C headers now (so <cmath> not <math.h>) unless
 there's a good reason.
  * Use str() not om_tostring() - the latter is being phased out.
  * Generally in .cc files we use "using namespace std;" rather than
 explicitly qualifying with std:: everywhere, as that's more readable.

 Using XAPIAN_VISIBILITY_DEFAULT on inlined API methods seems rather odd.
 I slightly wonder if we really want/need a Xapian API feature to convert
 from nautical miles to metres, but at least it is inlined so isn't adding
 more symbol relocations.

 Unserialisation errors should now throw !UnserialisationError rather than
 !NetworkError.  This patch catches !NetworkError in at least two places
 and rethrows as !InvalidArgumentError.  I think this catch and rethrow
 should just be removed now we have !UnserialisationError.

 I wonder if it would be better to specify in the documentation that the
 lat/long coordinates should use the WGS84 datum rather than risking adding
 to the sadly rather widespread misunderstanding that a lat/long coordinate
 pair alone specifies a geographical location.  WGS84 is what they almost
 inevitably will be in anyway given the prevalance of GPS, and means that
 if we actually later want to extend the geospatial support in a way which
 requires the datum, then we haven't tied our own hands.  Meanwhile users
 can ignore the datum, but it's then their problem not ours.

 What does code coverage look like?

-- 
Ticket URL: <http://trac.xapian.org/ticket/481#comment:2>
Xapian <http://xapian.org/>
Xapian