Expansion of Warp API bindings #508

metasim · 2023-12-28T21:36:53Z

I agree to follow the project's code of conduct.
I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

Extract replicated methods in *Options types into macros.
Figure out why warp::tests::test_create_reproject fails on GDAL < 3.4 due to statistics seemingly not respecting no-data values.

CHANGES.md

src/raster/warp/create_and_reproject.rs

lnicola · 2024-01-03T17:18:43Z

src/raster/warp/create_and_reproject.rs

+            driver.c_driver(),
+            ptr::null_mut(),
+            warp_options.resampling_alg().to_gdal(),
+            warp_options.memory_limit() as f64,


I wonder why this is double 😄.

src/raster/warp/create_and_reproject.rs

src/raster/warp/resample.rs

lnicola · 2024-01-03T17:25:25Z

src/raster/warp/warp_options.rs

+    /// Set the datatype used during processing.
+    ///
+    /// If unset, the algorithm picks the datatype.
+    pub fn with_working_datatype(&mut self, dt: GdalDataType) -> &mut Self {


Nit: do we use datatype or data_type?

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

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! 🤣

https://en.wiktionary.org/wiki/datatype

lnicola · 2024-01-03T17:26:08Z

src/raster/warp/warp_options.rs

+        let dt: GdalDataType = c_dt.try_into().ok()?;
+
+        // Default is `Unknown`, so we consider that "unspecified".
+        if dt == GdalDataType::Unknown {


This might be a little confusing, should we drop the Option?

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);

?

src/raster/warp/warp_options.rs

src/xml.rs

lnicola · 2024-01-03T17:36:03Z

src/xml.rs

+    }
+}
+
+impl Display for GdalXmlNode {


Should this also implement ToString?

I've tended to conflate Display and ToString (since Display is ToString). Any advice on how to decide?

Interesting... Docs say this:

lnicola · 2024-01-03T17:43:58Z

Looks pretty good, but I only checked the first two commits.

lnicola · 2024-01-03T19:14:41Z

I do wonder, though, if we shouldn't go straight for GDALWarp. It's not that much harder to use, and I think that mailing list reply suggested that it's the most powerful API.

metasim · 2024-01-03T20:42:02Z

I do wonder, though, if we shouldn't go straight for GDALWarp. It's not that much harder to use, and I think that mailing list reply suggested that it's the most powerful API.

Good point. I'll research. At first glance it looked like it required a bunch more work, but maybe I jumped to conclusions there.

lnicola · 2024-01-03T20:45:22Z

It's a lot of work to write the bindings for, it's similar to gdaldem.

…_reproject.

metasim force-pushed the feature/warp branch 2 times, most recently from 9e9ba71 to c6438bb Compare December 29, 2023 15:52