thinkingmachines / geowrangler

🌏 A python package for wrangling geospatial datasets
https://geowrangler.thinkingmachin.es/
MIT License
47 stars 14 forks source link

Add quadkey conversion to xy coordinates in the Bingtiles grid generator #206

Closed tm-abby-moreno closed 1 year ago

tm-abby-moreno commented 1 year ago

This PR is based on the replies in discussion #90

Added a boolean parameter add_xy_cols in the class BingTileGridGenerator whether user would want to convert quadkey to xy coordinates. The newly created function quad_to_xy is based on this stackexchange.

I tested this on zoom_level = 12 and speed is still the same! I then tested it on zoom_level = 16 because this was the use case for CI and the baseline (without quadkey conversion) is ~32 mins. and when there is quadkey conversion to xy it still runs for 30 mins. TLDR: the things that I added did not impact the runtime 😄 I also added to the tutorial and fixed some typos in other comments.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

tm-abby-moreno commented 1 year ago

Updated the code to call xyz directly and added a test @jtmiclat @butchtm :)

I took some time as I was testing it out. For zoom_level = 15 it runs for only ~13 mins. but when I tried it the first time for zoom_level = 16 it ran for ~50 mins. But I tried running again just this morning and it ran for ~32 mins. Just wanted to flag this! Not exactly sure what the problem is. Note: I always test it on 16 because this is the use-case for Josh in CI.

re: failing pre-commit test how do I fix this?

jtmiclat commented 1 year ago

@tm-abby-moreno. You can ignore the pre-commit test failing. I took a look and the issue isnt with the code you changed but a dependency issue in the pre-commit.

Addressed that issue in a separate PR: https://github.com/thinkingmachines/geowrangler/pull/208