jazzband / geojson

Python bindings and utilities for GeoJSON
https://pypi.python.org/pypi/geojson/
BSD 3-Clause "New" or "Revised" License
915 stars 121 forks source link

Initial commit of generating random data #60

Closed rowanwins closed 9 years ago

rowanwins commented 9 years ago

Hi @frewsxcv

As suggested here is a pull request for work initial work on generating random data.

Cheers Rowan

frewsxcv commented 9 years ago

If you want the commit to be tied to your GitHub username, you should follow these instructions:

https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/

Not mandatory, but might be nice to give you proper credit. Once you do that, you can just git commit --amend and git push -f to repush the commit.

frewsxcv commented 9 years ago

Also, if you have time, there are style/linting issues which you can see here:

https://travis-ci.org/frewsxcv/python-geojson/jobs/69054890

If you don't get around to them, I can fix them myself later

rowanwins commented 9 years ago

Hi @frewsxcv Thanks, I didn't realise you could actually see what was failing, now that you point it out its obvious! And I just installed a python linter so that helped :) I've still got one issue to do with line length given all my params, im not sure about the best way to handle that.

Anyway let me know how you're getting on, no rush of course Cheers Rowan

frewsxcv commented 9 years ago

Before:

 def generate_random(featureType, numberFeatures=1, numberVertices=3, boundingBox=[-180.0, -90.0, 180.0, 90.0]): 

After:

 +def generate_random(featureType, numberFeatures=1, numberVertices=3,
                      boundingBox=[-180.0, -90.0, 180.0, 90.0]): 
frewsxcv commented 9 years ago

More info about indentation from the official style guide

http://legacy.python.org/dev/peps/pep-0008/#indentation

rowanwins commented 9 years ago

Hi @frewsxcv

Sorry for the delay in progress, just a quick update, I've been a bit stuck on the random polygon generation, my original idea of simply sorting the coords failed (given hindsight that was kind of obvious!). The geojson-random library from mapbox contains code to generate random polys although Im having a hard time understanding the maths, I think I need to sit down with a piece of paper to fully understand whats going on.

Cheers Rowan

frewsxcv commented 9 years ago

If you want, we can merge in a basic implementation of the random geometry algorithm you have now, then when you understand the math more, open a new PR improving it

rowanwins commented 9 years ago

Hi @frewsxcv

well I think I've pretty got things working now at a basic level, hooray!

For the polygon creation I took some code from here although this code wasn't designed for dealing with lat lons so its not working with the bounding box properly yet, I still need to finalise the maths around it but it's at least working for the timebeing.

This linting business took a while I get my head around, I've think I've now got a decent linter installed so fingers crossed things are looking better now!

Anyway let me know what you think, I hope it forms a useful part of the library.

Cheers Rowan

frewsxcv commented 9 years ago

I haven't forgotten about this! I'll be returning home from my month long traveling in a couple days. Hopefully I'll look at this later this week

frewsxcv commented 9 years ago

@rowanwins Looks good! You're welcome to add yourself under the Credits in the README.md if you want before I merge this

rowanwins commented 9 years ago

Hi @frewsxcv , happy not to be credited

I do have one final question which it would be good to get your opinion on. I've been debating removing the multi-feature support. The current approach locks people into a geometrycollection and doest allow properties. It may be easier to let people write the loop themselves if they want multi-features. What do you think?

FYI I I just used this to generate 100,000 random points with properties in a feature collection using this approach, took ~5s to run. Very handy for test data!

frewsxcv commented 9 years ago

I think it's okay to leave out the multi-feature support; writing a loop should be simple enough, though it's up to you whether you want to keep it or not

FYI I I just used this to generate 100,000 random points with properties in a feature collection using this approach, took ~5s to run.

:D

rowanwins commented 9 years ago

@frewsxcv have removed the multi-feature support and re-committed, this is it I reckon...

frewsxcv commented 9 years ago

Looks great @rowanwins, thanks!

frewsxcv commented 9 years ago

Just released 1.3.0 that includes this change

https://pypi.python.org/pypi/geojson/1.3.0

Thanks again!