georust / gdal

Rust bindings for GDAL
https://crates.io/crates/gdal
MIT License
359 stars 94 forks source link

Add `RasterBand::write_block` #490

Closed lnicola closed 9 months ago

lnicola commented 10 months ago
lnicola commented 10 months ago

Will rebase on top of #489.

r? @metasim

metasim commented 9 months ago

Should we also support {read|write}_block without ndarray?

lnicola commented 9 months ago

Should we also support {read|write}_block without ndarray?

I think we should, but I don't think I want to do it in this PR, since there's a bit of design space to explore. I don't like the current situation either, but in the meanwhile it's probably better than having no access to these.

lnicola commented 9 months ago

@metasim do you ever use the MD API, and would you expect it to require ndarray, or use some form of Buffer3, BufferN version?

metasim commented 9 months ago

do you ever use the MD API, and would you expect it to require ndarray, or use some form of Buffer3, BufferN version?

@lnicola I do not. That's a good point (I didn't realize MD required it). For that API it seems reasonable to require ndarray, and would be silly to implement a bespoke multidimensional array capability just to avoid a dependency.

I wouldn't be adverse to ndarray being required for everything. But because we advertise it as an "optional" dependency, I don't think not using it should preclude access to basic functionality (i.e. block I/O).

lnicola commented 9 months ago

I didn't realize MD required it

It doesn't, I should have checked before asking :pensive:.

metasim commented 9 months ago

PS: I think ndarray is fantastic. My position is more about it's optionality, and treating Buffer as a first-class citizen in face of that optionality.