georust / gdal

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

Lift `ndarray` compatibility into `Buffer<T>` #494

Closed metasim closed 7 months ago

metasim commented 7 months ago

cc: @mwielocha

lnicola commented 7 months ago

I'll have to think a bit more about this one, but I'm not sure I'm a fan. We should probably enable conversion from Buffer to Array2 and back instead of having two separate read and write functions.

But more importantly, we should have a way to pass in a buffer instead of allocating a new one each time.

metasim commented 7 months ago

@lnicola Let me restate the use case just to make sure it's clear what I'm trying to do here.

I have an application where I do not want to include/use the ndarray library, and I want to be able to read an image block by block. Before this PR I could not do that. Currently, block reading is only available if you are an ndarray user.

The renaming of the existing one is clearly not required, so I can figure out some other naming if that's objectionable.

metasim commented 7 months ago

I just re-read your comment. If what you're arguing is that we should normalize the I/O around the Buffer<T> type (or, perhaps even better, Vec<T>!), then I'm even more of a fan of that, but wondered how controversial that would be.

Instantiating ArrayX<T> from a Vec<T> is so easy, it does seem silly to have dual methods.

lnicola commented 7 months ago

Yes, I think we should use Buffer with easy conversions to ndarray. It's better than Vec because it has the shape information.

metasim commented 7 months ago

I'll prototype something.