JuliaEarth/CoordRefSystems.jl

Missing coordinate conversions

ettersi opened this issue ยท 6 comments

CoordRefSystems currently does not implement a few coordinate conversions that I think it should.

julia> convert(Cartesian, WebMercator(0,0))
ERROR: MethodError: Cannot `convert` an object of type
  WebMercator{WGS84Latest, Quantity{Float64, ๐‹, Unitful.FreeUnits{(m,), ๐‹, nothing}}} to an object of type
  Cartesian

julia> convert(LatLon, LatLonAlt(0,0,0))
ERROR: MethodError: Cannot `convert` an object of type
  GeodeticLatLonAlt{WGS84Latest, Quantity{Float64, NoDims, Unitful.FreeUnits{(ยฐ,), NoDims, nothing}}, Quantity{Float64, ๐‹, Unitful.FreeUnits{(m,), ๐‹, nothing}}} to an object of type
  GeodeticLatLon

It would be great if these could be added. Thanks!

@ettersi , regarding the first conversion, you can convert to LatLonAlt first and then convert to Cartesian since these two methods are already implemented. Would you like to submit a PR?

The second conversion should be trivial to add as well in a second PR.

I've added the Cartesian <> Projected conversion in #52. Regarding the LatLon <> LatLonAlt conversion, I've since come to realise that implementing this as a Base.convert() method might be dangerous.

julia> Base.convert(::Type{LatLon}, coords::LatLonAlt{Datum}) where {Datum} = convert(LatLon{Datum}, coords)
       Base.convert(::Type{LatLon{Datum}}, coords::LatLonAlt{Datum}) where {Datum} = LatLon(coords.lat, coords.lon)

julia> a = Ref{LatLon}(LatLon(0,0))
       a[] = LatLonAlt(0,0,1e10)
       a[] 
# Where did my Alt go?!
GeodeticLatLon{WGS84Latest} coordinates
โ”œโ”€ lat: 0.0ยฐ
โ””โ”€ lon: 0.0ยฐ

Of course, one could argue that the user asked for this since they explicitly requested a Ref{LatLon}, but I'm not sure whether this argument strikes the right balance between supporting power users and protecting mere mortals from silly mistakes. Also, just for reference, the Julia docs say this about convert():

[convert()] is also usually lossless; converting a value to a different type and back again should result in the exact same value.

Yes, we saw this pattern before, and decided that the feature is good enough to ignore the convert footnote. The problem with convert is that it was designed with very strict computer-science use cases in mind. We could have used a different function name to avoid these pedantic footnotes, but then we would loose the nice syntax

Mercator[LatLon(0,0), LatLon(0,90), ...]

and all the nice automatic conversions that Julia does when it encounters two CRS of different type.

We could have used a different function name to avoid these pedantic footnotes, but then we would loose the nice syntax

Mercator[LatLon(0,0), LatLon(0,90), ...]

and all the nice automatic conversions that Julia does when it encounters two CRS of different type.

You can always have a separate conversion mechanism (e.g. constructors) which is free to add or remove information, and then make convert() dispatch to this mechanism whenever the conversion is lossless.

I don't remember out of my head now, but I think Julia works in the opposite direction, it uses convert as a fallback to construct a type to another. In any case, I believe that we are fine with adopting convert between LatLon and LatLonAlt.

Feel free to submit a PR for it. We can review and merge today, together with the other PR, and then release a patch.

Fixed on master.