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

use WGS84 as default CRS for GeoJSON outputs #165

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

vincentsarago
Copy link
Member

ref #164

@vincentsarago
Copy link
Member Author

@AndrewAnnex I'm not sure if you use the feature method but this will break the previous implementation.

That's said I'm making a minor version update because it basically follow the GeoJSON specification (coordinates should be in WGS84)

To support other geographic crs, users would need to use the geographic_crs option

morecantile/models.py Outdated Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
@AndrewAnnex
Copy link
Contributor

@vincentsarago okay responding to this one, yes I use all the capabilities of morecantile including the geojson output.

Broadly, I think since this is probably okay so long as exceptions don't get thrown preventing any non WGS84 stuff happening (warnings are good but I would want them to be opt-in for #166). As per my comment I believe however that exceptions would get thrown.

Now onto my rant for RFC7946: The spec says that although WGS84 is the assumed default, "alternative coordinate reference systems can be used..". There is no use of RFC2119 language in section 4 except for the z axis. Therefore, I think it's fine to continue producing non WGS84 geojson in this project and elsewhere, just don't force users to use WGS84 when they explicitly don't want it or implicitly don't want it by using a TMS that doesn't use WGS84.

Copy link
Contributor

@AndrewAnnex AndrewAnnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left two replies in above comments rather than view this pr review mechanism

morecantile/models.py Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

2 participants