search

Toblerity / rtree

Rtree: spatial index for Python GIS
https://rtree.readthedocs.io
MIT License
627 stars 123 forks source link

don't require file to be writeable if we're not going to write to it #281

Closed jason-curtis closed 1 year ago

jason-curtis commented 1 year ago

closes #247

hobu commented 1 year ago

Please include some tests so this behavior doesn't regress again.

jason-curtis commented 1 year ago

Unfortunately it turns out the same check is happening on the C library level as well: https://libspatialindex.org/en/latest/doxygen/DiskStorageManager_8cc_source.html#l00173

I'm hoping it's an overzealous check and write access isn't actually required but I can't be sure. In any case I'm not equipped to do a quick C side project right now so I'm going to work around the issue instead. I've checked in my latest implementation including (failing) unit tests; feel free to close this PR or take it over if you prefer.

jason-curtis commented 1 year ago

In fact, since the C code is already doing its own checks and has reasonable error messages, maybe we should just remove these checks entirely from the Python implementation?

mwtoews commented 1 year ago

[repeating from linked issue] You could raise an issue and/or PR for libspatialindex; the source code on GitHub is here. But yeah, I agree that a fix from the library level would be better.