Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LatLngAlt as default #17

Open
workingDog opened this issue Nov 3, 2015 · 3 comments
Open

LatLngAlt as default #17

workingDog opened this issue Nov 3, 2015 · 3 comments

Comments

@workingDog
Copy link

I'm starting to write a simple app to convert Kml to GeoJSON using the play-geojson library.
See https://github.com/workingDog/kmlToGeojson.

Given that the WGS84 as a CRS references the ellipsoid,
would it make sense to have the LatLng be a LatLngAlt with the altitude as an option as I did in my app?
At the moment the altitude cannot be set with just LatLng.

PS
The SphericalMercator in the readme.md shows Ints not Doubles and the installation instruction shows "com.typesafe.play.extras" %% "play-geojson" % "1.2.0"

@jroper
Copy link
Owner

jroper commented Nov 3, 2015

I think it would make sense to add a LatLngAlt type to play-geojson, but still keep LatLng around. In practice, many people simply don't care about altitude when working with play-geojson, so keeping a very simple LatLng around I think makes sense - for me, working with leaflet on the client side which also defines a LatLng type, it feels more natural to work with LatLng on the client and on the server. Note that the design of play-geojson makes no one CRS better than another, a user simply selects which one they want to use using the type system.

One other change that we probably should do to play-geojson is support reading LatLng from a 3 coordinate array, currently I think it will fail. It should simply drop the altitude if it encounters it. Or, we should at very least produce a meaningful error message, currently the user will just get a match error.

@workingDog
Copy link
Author

Ok, grab the code for LatLngAlt and add
case Seq(lng, lat, alt) => LatLngAlt(lat, lng)
to LatLng -> object Wgs84Format __.read.

@workingDog
Copy link
Author

correction, add case Seq(lng, lat, alt) => LatLng(lat, lng)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants