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

Make higher-zoom JSON tiles from upstream SharedStreets data #3

Merged
merged 8 commits into from
Mar 8, 2018

Conversation

migurski
Copy link
Collaborator

@migurski migurski commented Mar 7, 2018

I’ve started a sharedstreets package with a single module that generates a GeoJSON-compatible single file representation of street data. See #1.

Sample installation and usage:

> pip install .
> sharedstreets-tile 16 10509 25324 > 16-10509-25324.geojson

16-10509-25324.geojson contains sample output matching what we’ve been using internally for the past few months. I see that a few names like “fromIntersectionId” for “startIntersectionId” have changed along the way.

Geometries and intersections fit neatly into GeoJSON, while references go in a separate list.

I’d love to hear feedback on this. Next steps would be to add a basic preview and tile server.

@migurski
Copy link
Collaborator Author

migurski commented Mar 7, 2018

Also, I’m ignoring the metadata for now, but it could become useful if OSM street names appear there.

@DenisCarriere
Copy link
Contributor

Going to merge this in, however we should stick with Typescript (Javascript) for any tools.

If we ever want to build Web based tools, Python is not the way to go...

We should can develop basic Python implementations, however this seems to spread out the development across multiple languages (Python/Java/Javascript).

To develop sharedstreets-tile in NodeJS would be fairly simple.

We should develop sharedstreets-geojson in NodeJS (not Python) that way the library works in the browser & on the server as a CLI.

Metadata is a tough one, not too sure where I stand on that one.

CC: @kpwebb @migurski

@DenisCarriere DenisCarriere merged commit 6f0f210 into master Mar 8, 2018
@DenisCarriere
Copy link
Contributor

I see that a few names like “fromIntersectionId” for “startIntersectionId” have changed along the way.

@migurski A few property names have been adjusted, not too sure which version of the Proto file you are using.

Latest SharedStreets.proto

https://github.com/sharedstreets/sharedstreets-ref-system/blob/master/proto/sharedstreets.proto

@migurski migurski deleted the migurski/serve-tiles branch March 9, 2018 01:15
@migurski
Copy link
Collaborator Author

migurski commented Mar 9, 2018

Thanks for the merge!

Is your suggestion that we should halt development of this library?

@migurski migurski mentioned this pull request Mar 9, 2018
@kpwebb
Copy link
Member

kpwebb commented Mar 9, 2018

@DenisCarriere and @migurski We should maintain parity between the JS and Python libraries -- definitely don't think we can "replace" the python library with the JS version. There are different use cases for each library. I can see the JS versions getting used for validation and QA (acting as a reference lib for other languages), but doesn't diminish the need for multilingual support!

@migurski
Copy link
Collaborator Author

migurski commented Mar 9, 2018

Great, I’ll keep moving forward with this one. I’d like to get the GeoJSON output from #6 into shape as well; it still looks more like the internal Remix JSON we’re consuming than something truly general-purpose.

@DenisCarriere
Copy link
Contributor

The multilingual support will come for sure, it just seems like there's missing a lot of pieces on the Python library to be jumping into advanced tools right away.

Baby steps...

Ideally all of the NodeJS libraries will have CLI support that way we can easily "wrap" them into Python.

@kpwebb
Copy link
Member

kpwebb commented Mar 9, 2018

@DenisCarriere I'm not sure wrappers will work -- @migurski team at remix needs to be able to build product on top of Python version so things should unfold as needed on various languages in parallel. Also, CLI wrappers won't solve the ESRI/QGIS integration needs.

There's value in polynucleation -- we're still early and can learn things about the client library design driven by different use cases/users!

@kpwebb
Copy link
Member

kpwebb commented Mar 9, 2018

@migurski thanks for pushing on the GeoJSON spec -- the version you shared in December looked really useful, but as soon as you think it's ready for wider use let's look into integrating it into our AWS API Gateway workflow.

I'm interested in setting up Lambda's that translate this via the REST endpoint. This combined with your hierarchical tiles idea should make this very useful for simple client-side applications.

@migurski
Copy link
Collaborator Author

migurski commented Mar 9, 2018

Thanks Kevin; I’ll start a PR here with some edits to the GeoJSON sample to bring it in-line with the latest SharedStreets terminology. Also, just to confirm: wrappers would be a less-good approach. Shelling out introduces a process creation overhead, and requires an app to have NodeJS installed under the main language as well. I’d much prefer to access the data directly, and I think protobufs are a fine multi-language solution to that problem.

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.

3 participants