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

Change handling of EPSG/SRID to match standard #79

Merged
merged 2 commits into from
Jan 4, 2018

Conversation

bernardd
Copy link
Contributor

@bernardd bernardd commented Jan 3, 2018

PostGIS' ST_AsGeoJSON() function (http://www.postgis.net/docs/ST_AsGeoJSON.html) produces a CRS field with a colon between EPSG and the SRID integer, eg EPSG:4326. Googling around, this appears to be the standard way of doing things. The existing code expects no colon (EPSG4326). I've changed it to match what seems to be the standard.

The catch with this change is that anyone who's encoded a geom with the old code will not be able to decode with the new version. If you think that's likely to be an issue, it could be resolved by adding back in the old clause for get_srid/1 between the new version and the version that takes nil. I didn't do that in order to keep the code simple, but I have no problem with doing so.

@bryanjos
Copy link
Contributor

bryanjos commented Jan 3, 2018

@bernardd you are correct and it is a bug. To disrupt the least amount of people in the short term, could you add the function head that searches for the colon instead of replacing the existing one?

Something like:

defp get_srid(%{"type" => "name", "properties" => %{ "name" => "EPSG" <> srid } }) do
    {srid, _} = Integer.parse(srid)
    srid
end

defp get_srid(%{"type" => "name", "properties" => %{ "name" => "EPSG:" <> srid } }) do
    {srid, _} = Integer.parse(srid)
    srid
end

I think your change to add_crs should stay as is. That way any geoJSON created have the correct string

@bernardd
Copy link
Contributor Author

bernardd commented Jan 3, 2018

Yep, can do. The clauses have to be the other way around or else the non-colon version will also catch inputs with the colon.

@bryanjos bryanjos merged commit 49f5e24 into felt:master Jan 4, 2018
@bryanjos
Copy link
Contributor

bryanjos commented Jan 4, 2018

Looks good! Thanks!

@bernardd bernardd deleted the handle-epsg-colon branch January 4, 2018 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants