Expansion of Warp API bindings #508
Conversation
metasim
force-pushed
the
feature/warp
branch
2 times, most recently
from
9e9ba71
to
c6438bb
Compare
| driver.c_driver(), | ||
| ptr::null_mut(), | ||
| warp_options.resampling_alg().to_gdal(), | ||
| warp_options.memory_limit() as f64, |
| /// Set the datatype used during processing. | ||
| /// | ||
| /// If unset, the algorithm picks the datatype. | ||
| pub fn with_working_datatype(&mut self, dt: GdalDataType) -> &mut Self { |
There was a problem hiding this comment.
Nit: do we use datatype or data_type?
There was a problem hiding this comment.
We're not consistent by any stretch:
❯ rg --count-matches --ignore-case datatype --glob '*.rs' src
src/driver.rs:12
src/errors.rs:2
src/raster/tests.rs:6
src/raster/types.rs:177
src/raster/mod.rs:4
src/raster/rasterize.rs:1
src/raster/rasterband.rs:9
src/raster/mdarray.rs:76
❯ rg --count-matches --ignore-case 'data[-_]type' --glob '*.rs' src
src/raster/mdarray.rs:25
src/errors.rs:2
src/driver.rs:2
src/raster/types.rs:3
There was a problem hiding this comment.
I have a bias toward the one word "datatype" form.
Since Wiktionary is our authoritative source for justifying our own spelling biases, I think I'm justified! 🤣
| let dt: GdalDataType = c_dt.try_into().ok()?; | ||
|
|
||
| // Default is `Unknown`, so we consider that "unspecified". | ||
| if dt == GdalDataType::Unknown { |
There was a problem hiding this comment.
This might be a little confusing, should we drop the Option?
There was a problem hiding this comment.
If we did, would we change this
let dt: GdalDataType = c_dt.try_into().ok()?;to
let dt: GdalDataType = c_dt.try_into().unwrap_or(GdalDataType::Unknown);?
| } | ||
| } | ||
|
|
||
| impl Display for GdalXmlNode { |
There was a problem hiding this comment.
Should this also implement ToString?
There was a problem hiding this comment.
I've tended to conflate Display and ToString (since Display is ToString). Any advice on how to decide?
There was a problem hiding this comment.
Interesting... Docs say this:
|
Looks pretty good, but I only checked the first two commits. |
|
I do wonder, though, if we shouldn't go straight for |
Good point. I'll research. At first glance it looked like it required a bunch more work, but maybe I jumped to conclusions there. |
|
It's a lot of work to write the bindings for, it's similar to gdaldem. |
metasim
force-pushed
the
feature/warp
branch
4 times, most recently
from
b3d68e3
to
a16e93b
Compare
|
Recently I've been experimenting with using gdal-sys to make calls to GDALWarp, which I've finally gotten running to my liking (besides the pesky MEM format..). But of course, only afterwards did I stumble across this PR Anyway, getting GDALWarp implemented into the gdal crate would be a big win, IMO. So, if it helps, I would be happy to push this topic forward a bit with my learnings from the past few days? Please let me know :) |
|
@sevberg I'm happy for you to take over this branch. I've had to change jobs to something outside of the geospatial field, and sadly no longer have time to focus on this (awesome) project. Let me know if you have any questions. |
CHANGES.mdif knowledge of this change could be valuable to users.*Optionstypes into macros.warp::tests::test_create_reprojectfails on GDAL < 3.4 due to statistics seemingly not respecting no-data values.