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

Match functionality provided by the javascript library #35

Merged
merged 8 commits into from
Dec 10, 2017

Conversation

jguthmiller
Copy link

This PR aims to improve interoperability between the python and javascript implementations. There is still a compatibility issue with encoding/decoding custom properties on features, but that fix will most likely need to be made on the javascript side.

@jlaine
Copy link
Contributor

jlaine commented Sep 25, 2017

Hi Jake! I'm guessing the maintainers will find this PR hard to review as it mixes a number of unrelated code changes.

As a starting point : maybe the PEP8 changes could go in first (with no behaviour change) in a separate PR. It might be worth adding a flake8 run to the test suite too.

@jlaine
Copy link
Contributor

jlaine commented Sep 25, 2017

Removing topobuf support seems fair, as it was removed from the JS implementation over two years ago:

mapbox/geobuf@b4ba6ef

The README file needs updating though.

@sgillies
Copy link
Contributor

sgillies commented Oct 6, 2017

@jguthmiller this project isn't actively maintained and I'm not able to take on a 2.0 release. If pygeobuf is important to your work, we could discuss transferring the stewardship of this project and its name on PyPI to you and you could take the lead for 2.0. What do you think about that @mourner?

@mourner
Copy link
Contributor

mourner commented Oct 6, 2017

I would love to transfer the ownership to someone who depends on this module! At first I made pygeobuf purely as a proof of concept before attempting a full JS reimplementation simply because Python had good protobuf bindings at the time, but I only intend to maintain the JavaScript version now.

@jlaine
Copy link
Contributor

jlaine commented Oct 6, 2017

Sounds good, Jake how about co-maintaining the project together?

@jguthmiller
Copy link
Author

The organization I work for (mySidewalk) would definitely be interested in continuing maintenance of this project. We're running a javascript frontend and a python backend, so it'll be in our best interests to make sure that both libraries continue to play well together.

@sgillies
Copy link
Contributor

@jguthmiller that's a very nice offer. I'm going to check our project hand-over checklist and report back here with next steps.

@sgillies
Copy link
Contributor

@jguthmiller can you give me a PyPI username that you would like to use to upload packages to https://pypi.python.org/pypi/geobuf ? I'll add the required role.

After the repo is handed over, you can set the username and password in the Travis CI settings to automate releases as I recently did in this project.

untitled

@jlaine
Copy link
Contributor

jlaine commented Oct 12, 2017

Same here @wemap uses pygeobuf to store pre-clustered data in tile form, which is why I've been offering to maintain the package for over 6 months.

@jlaine
Copy link
Contributor

jlaine commented Nov 8, 2017

Hi Jake, would you mind splitting out the PEP8 changes so that we can merge those first?

@jlaine
Copy link
Contributor

jlaine commented Nov 27, 2017

Hi Jake!

I've merged your PEP8 changes, could you possibly rebase this pull request ?

@jguthmiller
Copy link
Author

Sorry for the wait, but the PR has been rebased.

@jlaine
Copy link
Contributor

jlaine commented Dec 7, 2017

Awesome thanks Jake! I'm quite happy to merge as is but the README could do with an update to remove references to topojson

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.

None yet

4 participants