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

New module @turf/projection #927

Merged
merged 11 commits into from
Sep 5, 2017
Merged

New module @turf/projection #927

merged 11 commits into from
Sep 5, 2017

Conversation

stebogit
Copy link
Collaborator

@stebogit stebogit commented Sep 1, 2017

This is a first draft of the module(s) to convert any GeoJSON from WGS84 to Mercator and vice versa, where the conversion functions are taken from sphericalmercator.
Example:

var pt = turf.point([4, 7.5])
var mercator = turf.toMercator(pt);
var wgs84 = turf.toWgs84(mercator);

Do we actually want two separate modules for the conversions or a single one with a parameter?
We might have something like:

var pt = turf.point([4, 7.5])
var mercator = turf.convert(pt, 'wgs84', 'mercator');
var wgs84 = turf.convert(pt, 'mercator', 'wgs84');

I decided to use proj4 as a reference to create the test cases, is that ok?

Ref: #141, #925

@DenisCarriere
Copy link
Member

👍 Nice! Looking forward to play with this module 😄

I'm good for using toWgs84 and toMercator, these coordinate conversions should only be used for WGS84 => Mercator => WGS84, anything else more should be handle with custom scripts and not be built-in for TurfJS. The last thing we want is handling projection issues.

I decided to use proj4 as a reference to create the test cases, is that ok?

Proj4 is perfect! I would've used the same.

@DenisCarriere
Copy link
Member

DenisCarriere commented Sep 1, 2017

Significant recommendation, I would group these methods (toWgs84 & toMercator) in a single module (ex: @turf/projection) and have this module export those two or more methods.

Or we use your 2nd approach, except change convert to projection.

var projection = require('@turf/projection');
var pt = turf.point([4, 7.5])
var mercator = turf.projection(pt, 'wgs84', 'mercator');
var wgs84 = turf.projection(pt, 'mercator', 'wgs84');

@stebogit
Copy link
Collaborator Author

stebogit commented Sep 1, 2017

I would group these methods (toWgs84 & toMercator) in a single module (ex: @turf/projection) and have this module export those two or more methods.

This is what I really wanted to do, but I couldn't figure out how to do it, cause I didn't think about that!! 🎉
Excellent, will do! 😃 👍

@DenisCarriere
Copy link
Member

Awesome! 👍 Just need to look at @turf/helpers or @turf/meta for multi-function modules.

- merged toMercator and toWgs84 to 2turf/projection;
- added tests (180th meridian still fails);
- updated readme and bench;
@stebogit
Copy link
Collaborator Author

stebogit commented Sep 2, 2017

@DenisCarriere I am not sure how to handle the 180th meridian issue here: proj4 returns values quite different than sphericalmercator, thus the test fails.
What should be the output for the test case?
Is there anything like geojson.io for Mercator coordinates?

@stebogit stebogit changed the title New module(s) for projection conversion New module @turf/projection Sep 2, 2017
@stebogit stebogit added this to the 4.7.0 milestone Sep 3, 2017
@DenisCarriere DenisCarriere merged commit 9ba198e into master Sep 5, 2017
@DenisCarriere DenisCarriere deleted the projection branch September 5, 2017 03:44
@DenisCarriere
Copy link
Member

🎉 @stebogit module published @turf/projection

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

Successfully merging this pull request may close these issues.

2 participants