search

holoviz / spatialpandas

Pandas extension arrays for spatial/geometric operations
BSD 2-Clause "Simplified" License
308 stars 24 forks source link

Pass storage options, add some typing, minor clean up #78

Closed brl0 closed 3 years ago

brl0 commented 3 years ago

@jbednar,

First, thanks for your amazingly quick review!

Since your review, I've made some minor changes (as has @iameskild in a companion PR #79). One notable change of behavior is that I added an overwrite option for parquet output like dask uses as a guard against deleting the destination directory. Currently, if one were to write to an existing destination, even something like the root directory of a bucket or filesystem, spatialpandas calls filesystem.rm(path, recursive=True), which seems dangerous.
I wanted to make sure you are comfortable with this before merging, since it is potentially a slight change of behavior for existing users.

Also, once this PR and #79 are merged, I was planning to cut a new release. Let me know if you have any questions, concerns, etc.

Thanks again!

jbednar commented 3 years ago

The speed of my review is a complex function of what I happen to be in the middle of when it appears, plus how tricky the changes are! In this case it all looks good. I agree with the overwrite flag; seems like a very useful addition. And happy to see a new release when it's done!