codingsince1985 / geo-golang Goto Github PK

Sometimes we need to specify some more detailed query params. For example here we have a lack of language and region parameters for google geocoding api

It would be nice if the library could parse the various mapping providers results into an Address struct or something to take away the work for clients. Something as simple as parsing it into the following:

type Address struct {
	StreetAddress string
	City string
	State string
	Country string
	PostalCode string
}

This makes it easier to consume the API instead of doing this ourselves which is more error prone.

Seeing another panic, not sure if it is tracked or not.

I think it's this line:
func (r *geocodeResponse) Location() geo.Location { return r.Results[0].Locations[0].LatLng }

panic: runtime error: index out of range

goroutine 15 [running]:
panic(0x3a0bc0, 0xc420016100)
/usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:500 +0x1a1
../codingsince1985/geo-golang/mapquest/open.(*geocodeResponse).Location(0xc4204323a0, 0x6d, 0x5d32e0)
/Users/saheljalal/code/go/src/../codingsince1985/geo-golang/mapquest/open/geocoder.go:46 +0x47
../codingsince1985/geo-golang.HTTPGeocoder.Geocode.func1(0x5d4920, 0xc420017700, 0x448aa8, 0xc42014cbd0, 0x2e, 0xc420302d90)
/Users/saheljalal/code/go/src/../codingsince1985/geo-golang/http_geocoder.go:54 +0xbe
created by ../codingsince1985/geo-golang.HTTPGeocoder.Geocode
/Users/saheljalal/code/go/src/../codingsince1985/geo-golang/http_geocoder.go:55 +0xb6

Hi, I noticed that in the osm.Address struct, there is not hemlet field, if you see the response of https://nominatim.openstreetmap.org/reverse?format=xml&lat=43.0280986&lon=-78.8136961&zoom=18&addressdetails=1 there is a hemlet field in the address object, can we add Hemlet to osm.Address struct and also add it to func (a Address) Locality() string as the last field to check for locality?

Testing would be easier to perform and more reliable if the library used mock http responses for geo APIs (e.g. the API's seem to be a little flakey sometimes causing intermittent build failures).

We could record HTTP requests/responses from the existing tests as the initial mock http responses.

We could always conditionally run the tests in "integration" mode which would run the mock tests and actual API tests if we still wanted to exercise the live APIs.

Hi, I need to use a custom installation of Nominatim, so I added a function to pass the base URL in the package openstreetmap (pull request #22 )

BTW, thanks for this package, is very useful. I was using mapbox before, and now I'm moving to Nominatim without any hassle 👍

Hi and thanks for a nice framework. I've forked it and did a minor update to at least return city, country (and added GlobalCode) in google lookups.

Also added context to the api calls since I use those in lambdas and need to cancel when lambda timeout. I also need the context in the logger (extracts certain values from context when logging).

I've done this in the following fork: https://github.com/mariotoffia/geo-golang - if it is ok - I can create a pr (of which I revert back my renaming of package to yours).

Again, many thanks!

Cheers,
Mario :)

The tests are failing for opencage:

--- FAIL: TestGeocode (0.61s)
    geocoder_test.go:17: TestGeocode() failed <nil> {-37.8142175 144.9631608}
FAIL
coverage: 30.2% of statements in ./...
FAIL    github.com/codingsince1985/geo-golang/opencage  1.368s

Expected Lat Long:

• Expected: (-37.814218, 144.963161) Actual: (-37.8142175 144.9631608)
• Also sometimes returns Actual: {-37.8142176 144.9631608}

From what I can tell on google maps that's basically the same lat,long coordinate. We can update the test to ensure expected results are within a precision delta.

Hello,

By default the package does not include headers related to language, ie Accept-Language

Using, Accept-Language = en for example will result in the full name of the city/state

While, Accept-Language = EN for example will result in abbreviation for the city/state

In my setup I often see the later being returned which results in abbreviations for city/state.

Can we introduce an optional param to Geocoder to pass in headers when making a request to Nominatim (so that we can control the Accept-Language header)?

Thank you!

More about this issue is outlined here: osm-search/Nominatim#783

Some providers do give certain details about what type of result it is. Having this information (when available) will help validate the results, and can help determine if I need to use another provider as a fallback if the results is not accurate enough.

I only want to use the lib only to autocomplete adresses. I want to search for "Fred-Grube-Platz 23, 30451 Hannover" and get the "formattedAdress" back. IS that possible? Or is only a conversion from adress to lat/lon possible?

It would be nice if Geocoder was just an interface, it would allow implementations like:

• ChainedGeocoder - that's made up of a list of Geocoders that it will "fallback" on if one of them fails
• CachedGeocder - cache responses from API's using a cache provider in memory or redis etc
  • cache provider: https://github.com/patrickmn/go-cache

Geocoder interface

// Geocoder can look up (lat, long) by address and address by (lat, long)
type Geocoder interface {
    Geocode(address string) (Location, error)
    ReverseGeocode(lat, lng float64) (string, error)
}

ChainedGeocoder

type chainedGeocoder struct {
    Geocoders []geo.Geocoder
}

// Geocoder creates a chain of Geocoders to lookup address and fallback on
func Geocoder(geocoders ...geo.Geocoder) geo.Geocoder {
    return chainedGeocoder{Geocoders: geocoders}
}

CachedGeocoder

type cachedGeocoder struct {
    Cache cache.Cache     
    Geocoder geo.Geocoder
}

// Geocoder creates a chain of Geocoders to lookup address and fallback on
func Geocoder(geocoder geo.Geocoder) geo.Geocoder {
    return cachedGeocoder{Geocoder: geocoder}
}

Then you can stitch it all together:

func main() {
    // Create a cache with a default expiration time of 5 minutes, and which
    // purges expired items every 30 seconds
    c := cache.New(5*time.Minute, 30*time.Second)
    chainedGeocoder := chained.Geocoder(
        // Google Maps
        cached.Geocoder(c, google.Geocoder(googleAPIKey)),
        // MapQuest Nominatim
        cached.Geocoder(c, nominatim.Geocoder("MAPQUEST_KEY")),
        // MapQuest Open
        cached.Geocoder(c, open.Geocoder("MAPQUEST_KEY")),
        //...
    )
    try(chainedGeocoder)
}

func try(geocoder geo.Geocoder) {
    location, _ := geocoder.Geocode(addr)
    fmt.Printf("%s location is %v\n", addr, location)
    address, _ := geocoder.ReverseGeocode(lat, lng)
    fmt.Printf("Address of (%f,%f) is %s\n\n", lat, lng, address)
}

https://goreportcard.com/report/github.com/codingsince1985/geo-golang#golint

$ golint ./...
geocoder.go:16:5: var timeoutInSeconds is of type time.Duration; don't use unit-specific suffix "Seconds"
mapbox/geocoder.go:14:3: don't use underscores in Go names; struct field Place_Name should be PlaceName
mapquest/nominatim/geocoder.go:13:2: don't use underscores in Go names; struct field Display_Name should be DisplayName
openstreetmap/geocoder.go:14:2: don't use underscores in Go names; struct field Display_Name should be DisplayName

I just checked the openstreetmap response parser, and when the returned longitude and latitude are empty, it returns nil for both location and error.

Returning nil as a valid value when there is no error is undesirable. It means that we have to check the return value, regardless of whether we have a return error or not.

Instead, it would probably be good to return a sentinel error, such as ErrNoCoordsFound.

Would you accpet a PR, or consider modifying this module such that users could pass a context to the Geocoder interface?
This addition would make this module much more powerful in production, since web servers would be able to pass the top level context for things like tracing and etc. The new interface would not be backwards compatible, but would be trivial for users to update, i.e. provide context.TODO to their calls as you've done

It seems that the Mapbox Geocoding API will occasionally return the address field of the result as a literal number instead of the expected string encoded number (as per this link). This causes problems for the json decoder which is expecting a string.

Here's a test case to see what I am talking about

package main_test

import (
	"testing"

	"github.com/codingsince1985/geo-golang/mapbox"
)

var key string = "<your-key-here>"

func TestReverseGeocode(t *testing.T) {
	g := mapbox.Geocoder(key)
	_, err := g.ReverseGeocode(-33.932933, 150.865816)
	if err != nil {
		t.Error("Got an error:", err)
	}
}

I've filed a bug with Mapbox but as this may happen for more points it may be worthwhile to fix/handle in this library. At the very least it would be worthwhile to keep track of this for future devs running into this problem.

I'm getting some odd results like getting a nil address or location when geocoding or reverse geocoding.

Geocoding "4000 Blackhawk Plaza Cir, Danville, California 94506, United States":
[Here] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.7999631 -121.9202164}
[OpenCage] Failed to geocode address: geocoding error: missing API key
[OpenStreetMap] Failed to geocode address: location is nil
[Bing] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.800872802734375 -121.9173355102539}
[Google] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.8006389 -121.919588}
[Nominatim] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{0 0}
[Open] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{39.78373 -100.445882}
[MapBox] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.800639 -121.919588}
[LocationIQ] Failed to geocode address: location is nil

Reverse Geocoding "37.8006389,-121.919588":
[Google] Failed to reverse geocode location: address is nil
[Here] Reverse geocoded location 37.8006389,-121.919588 to 'Danville, CA 94506, United States'
[Open] Reverse geocoded location 37.8006389,-121.919588 to 'Blackhawk Plaza Circle, 94506, Blackhawk, CA, US'
[MapBox] Reverse geocoded location 37.8006389,-121.919588 to '4000 Blackhawk Plaza Cir, Danville, California 94506, United States'
[OpenStreetMap] Reverse geocoded location 37.8006389,-121.919588 to '4014, Blackhawk Plaza Circle, Blackhawk, Contra Costa County, California, 94506, United States of America'
[Bing] Reverse geocoded location 37.8006389,-121.919588 to 'Street, Danville, CA 94506'
[OpenCage] Failed to reverse geocode location: geocoding error: missing API key
[LocationIQ] Reverse geocoded location 37.8006389,-121.919588 to '4014, Blackhawk Plaza Circle, Blackhawk, Contra Costa County, California, 94506, United States of America'
[Nominatim] Reverse geocoded location 37.8006389,-121.919588 to 'Blackhawk Plaza Circle, Blackhawk, Contra Costa County, California, 94506, United States of America'

I'm not sure if I'm doing something wrong or my expectation is incorrect but I would imagine we should get an error if an address or location is nil.

I have a test that uses lat/long 0,0 and it seems to panic the library.

panic: runtime error: index out of range

goroutine 76 [running]:
panic(0x39cd00, 0xc4200140f0)
/usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:500 +0x1a1
../vendor/github.com/codingsince1985/geo-golang/mapquest/open.(*geocodeResponse).Address(0xc42043c5c0, 0x50, 0x5cd2e0)
../vendor/github.com/codingsince1985/geo-golang/mapquest/open/geocoder.go:49 +0x18a
../vendor/github.com/codingsince1985/geo-golang.HTTPGeocoder.ReverseGeocode.func1(0x5ce920, 0xc4200156f0, 0x444178, 0x0, 0x0, 0xc42013b3e0)
../vendor/github.com/codingsince1985/geo-golang/http_geocoder.go:71 +0xaa
created by ../vendor/github.com/codingsince1985/geo-golang.HTTPGeocoder.ReverseGeocode
../vendor/github.com/codingsince1985/geo-golang/http_geocoder.go:72 +0xb7
exit status 2

It would be potentially useful to indicate in some way which provider successfully returned data. If the library doesn't construct a proper address for example while reverse geocoding, that info can at least be used to help parse the returned string.

Hi,

Can you create a new release tag since the last one is from 2019? Thanks!

Cleanup test warnings:

warning: no packages being tested depend on github.com/codingsince1985/geo-golang/bing
warning: no packages being tested depend on github.com/codingsince1985/geo-golang/google
warning: no packages being tested depend on github.com/codingsince1985/geo-golang/here
warning: no packages being tested depend on github.com/codingsince1985/geo-golang/mapbox
warning: no packages being tested depend on github.com/codingsince1985/geo-golang/mapquest/nominatim
warning: no packages being tested depend on github.com/codingsince1985/geo-golang/mapquest/open
warning: no packages being tested depend on github.com/codingsince1985/geo-golang/opencage

We just need to filter out packages that aren't direct dependencies using some go list -f magic

• http://dave.cheney.net/2014/09/14/go-list-your-swiss-army-knife

When running the client in a resilient / HA setup on multiple instances simultaneously, having an in-memory-cache might be suboptimal. A centralised cache (redis, memcached, etc) would do a better job to reduce calls to the provider. So here's a proposal :)
How about converting cachedGeocoder.Cache to:

type Cache interface {
    Get(key string) (interface{}, error)
    Set(key string, value interface{}) error
}

Currently the _type property is not included in the returned Address struct. Any ways to add it?

Looking at codecov.io seems to be a more flexible more full featured solution to code coverage than coveralls.io.

• Provides more nuanced diff of coverage (coveralls.io seems broken and confusing in that regard, although they did claim to fix it).
• Provides graphs of code coverage over time
• Provide file and tree view to view coverage by package/directory of packages not just per file.

For instance for address:
31171 Fisher Manor,Simeonbury,United States

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1228a40]

Because func response from http_geocoder.go returns nil when body is empty.

Any ideas if it's possible to run this in the browser as a service or service worker? Can you compile this package to web assembly/WASM?

Hi, I'm looking at using this library in a project. Please review the results of my get command. Maybe I'm doing something wrong.

user@desktop:~/go/src/photos$ go get -t github.com/codingsince1985/geo-golang
go: downloading github.com/codingsince1985/geo-golang v1.8.1
go: github.com/codingsince1985/geo-golang upgrade => v1.8.1
# github.com/codingsince1985/geo-golang
/home/user/go/pkg/mod/github.com/codingsince1985/[email protected]/geocoder.go:38:32: undefined: io.Discard
/home/user/go/pkg/mod/github.com/codingsince1985/[email protected]/http_geocoder.go:128:15: undefined: io.ReadAll

According to suggestions in my IDE (GoLand), maybe some "io" imports should be "io/ioutil"?

I appreciate any help

More often one would like to have an address, returned by reverse geocoding service, as a structure, with separate fields for all the parts an address consists of. Returning a formatted address doesn't cut it most of the times. Also (as a personal opinion of mine and engineers in my team) I find it nice having homogeneous definitions - Geocode returns the Location structure, so why not have a similar signature for ReverseGeocode.
Hence I would like to propose (and implement it all over) the following change in the Geocoder interface definitions:

type Geocoder interface {
	Geocode(address string) (*Location, error)
	ReverseGeocode(lat, lng float64) (*Address, error)
}

with Address being defined in the package geo in a similar manner to the Location one

// Address is the structured representation of an address, including its flat representation
type Address struct {
	FormattedAddress string
	Street           string
	HouseNumber      string
	Suburb           string
	Postcode         string
	State            string
	StateDistrict    string
	County           string
	Country          string
	CountryCode      string
	City             string
}

My reasoning for the further proposed changes:

• Return pointer to the defined structure, so that the caller can check for nil; checking for zero values to deduce ErrNoResult may often go wrong (further no result being returned shouldn't be an error)
• Add second return value of type error makes is easier to catch errors and propagate them back to the caller, next to a nil, who then should decide how to react
• The (uniform) address string, currently returned by Address() would be accessible through the FormattedAddress field

Now that go modules seem to be the way / mainstream for dependency management, wouldn't it be good to also start using it?
I think a good first (and intermediate) step would be to start tagging releases in the established / required fashion (e.g. v1.5.0 as a suggestion straight away). This would also help people like me who have dependency to geo-golang and are using go modules to build their software.
Next step - define a module.
3. Profit 😂