From 0916638a6b82662c92b6d9256769168c041cf35b Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Sun, 13 Nov 2022 17:08:42 -0500 Subject: [PATCH 1/6] Fixes leakage of GDALDataType::Type in RasterBand. --- CHANGES.md | 4 ++++ gdal-sys/src/lib.rs | 6 ++++++ src/driver.rs | 12 ++++++------ src/raster/rasterband.rs | 22 +++++++++++----------- src/raster/tests.rs | 15 +++++++-------- src/raster/types.rs | 4 +++- 6 files changed, 37 insertions(+), 26 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 457fa801..7c571b2a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ ## Unreleased +- **Breaking**: `RasterBand::band_type` returns `GdalDataType` enum instead of `GDALDataType::Type` ordinal. Fixes [#333](https://github.com/georust/gdal/issues/333) + + - <> + ## 0.14 - Added new content to `README.md` and the root docs. diff --git a/gdal-sys/src/lib.rs b/gdal-sys/src/lib.rs index 120cec75..1fb403a5 100644 --- a/gdal-sys/src/lib.rs +++ b/gdal-sys/src/lib.rs @@ -3,5 +3,11 @@ #![allow(non_snake_case)] #![allow(clippy::upper_case_acronyms)] #![allow(rustdoc::bare_urls)] +// bindgen test code generates lots of warnings when testing +// sizes of types due to using constructs such as: +// `unsafe { &(*(::std::ptr::null::<__sbuf>()))._base as *const _ as usize }` +// This disables those warnings. +#![cfg_attr(test, allow(deref_nullptr))] + include!(concat!(env!("OUT_DIR"), "/bindings.rs")); include!(concat!(env!("OUT_DIR"), "/docs_rs_helper.rs")); diff --git a/src/driver.rs b/src/driver.rs index 8328d0b7..639864d7 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -93,12 +93,12 @@ impl Driver { /// ```rust, no_run /// # fn main() -> gdal::errors::Result<()> { /// use gdal::DriverManager; - /// use gdal::raster::GdalType; + /// use gdal::raster::GdalDataType; /// let d = DriverManager::get_driver_by_name("MEM")?; /// let ds = d.create("in-memory", 64, 64, 3)?; /// assert_eq!(ds.raster_count(), 3); /// assert_eq!(ds.raster_size(), (64, 64)); - /// assert_eq!(ds.rasterband(1)?.band_type(), u8::gdal_ordinal()); + /// assert_eq!(ds.rasterband(1)?.band_type(), GdalDataType::UInt8); /// # Ok(()) /// # } /// ``` @@ -122,12 +122,12 @@ impl Driver { /// ```rust, no_run /// # fn main() -> gdal::errors::Result<()> { /// use gdal::DriverManager; - /// use gdal::raster::GdalType; + /// use gdal::raster::GdalDataType; /// let d = DriverManager::get_driver_by_name("MEM")?; /// let ds = d.create_with_band_type::("in-memory", 64, 64, 3)?; /// assert_eq!(ds.raster_count(), 3); /// assert_eq!(ds.raster_size(), (64, 64)); - /// assert_eq!(ds.rasterband(1)?.band_type(), f64::gdal_ordinal()); + /// assert_eq!(ds.rasterband(1)?.band_type(), GdalDataType::Float64); /// # Ok(()) /// # } /// ``` @@ -155,7 +155,7 @@ impl Driver { /// # fn main() -> gdal::errors::Result<()> { /// use gdal::DriverManager; /// use gdal::raster::RasterCreationOption; - /// use gdal::raster::GdalType; + /// use gdal::raster::GdalDataType; /// use gdal::spatial_ref::SpatialRef; /// let d = DriverManager::get_driver_by_name("BMP")?; /// let options = [ @@ -168,7 +168,7 @@ impl Driver { /// ds.set_spatial_ref(&SpatialRef::from_epsg(4326)?)?; /// assert_eq!(ds.raster_count(), 1); /// assert_eq!(ds.raster_size(), (64, 64)); - /// assert_eq!(ds.rasterband(1)?.band_type(), u8::gdal_ordinal()); + /// assert_eq!(ds.rasterband(1)?.band_type(), GdalDataType::UInt8); /// assert_eq!(ds.spatial_ref()?.auth_code()?, 4326); /// # Ok(()) /// # } diff --git a/src/raster/rasterband.rs b/src/raster/rasterband.rs index 1efe22c7..b1f725d5 100644 --- a/src/raster/rasterband.rs +++ b/src/raster/rasterband.rs @@ -1,14 +1,13 @@ use crate::dataset::Dataset; use crate::gdal_major_object::MajorObject; use crate::metadata::Metadata; -use crate::raster::GdalType; +use crate::raster::{GdalDataType, GdalType}; use crate::utils::{_last_cpl_err, _last_null_pointer_err, _string}; use gdal_sys::{ self, CPLErr, GDALColorEntry, GDALColorInterp, GDALColorTableH, GDALComputeRasterMinMax, - GDALCreateColorRamp, GDALCreateColorTable, GDALDataType, GDALDestroyColorTable, - GDALGetPaletteInterpretation, GDALGetRasterStatistics, GDALMajorObjectH, GDALPaletteInterp, - GDALRIOResampleAlg, GDALRWFlag, GDALRasterBandH, GDALRasterIOExtraArg, GDALSetColorEntry, - GDALSetRasterColorTable, + GDALCreateColorRamp, GDALCreateColorTable, GDALDestroyColorTable, GDALGetPaletteInterpretation, + GDALGetRasterStatistics, GDALMajorObjectH, GDALPaletteInterp, GDALRIOResampleAlg, GDALRWFlag, + GDALRasterBandH, GDALRasterIOExtraArg, GDALSetColorEntry, GDALSetRasterColorTable, }; use libc::c_int; use std::ffi::CString; @@ -230,10 +229,10 @@ impl<'a> RasterBand<'a> { /// ```rust, no_run /// # fn main() -> gdal::errors::Result<()> { /// use gdal::Dataset; - /// use gdal::raster::{GdalType, ResampleAlg}; + /// use gdal::raster::{GdalDataType, ResampleAlg}; /// let dataset = Dataset::open("fixtures/m_3607824_se_17_1_20160620_sub.tif")?; /// let band1 = dataset.rasterband(1)?; - /// assert_eq!(band1.band_type(), u8::gdal_ordinal()); + /// assert_eq!(band1.band_type(), GdalDataType::UInt8); /// let size = 4; /// let mut buf = vec![0; size*size]; /// band1.read_into_slice::((0, 0), band1.size(), (size, size), buf.as_mut_slice(), Some(ResampleAlg::Bilinear))?; @@ -300,10 +299,10 @@ impl<'a> RasterBand<'a> { /// ```rust, no_run /// # fn main() -> gdal::errors::Result<()> { /// use gdal::Dataset; - /// use gdal::raster::{GdalType, ResampleAlg}; + /// use gdal::raster::{GdalDataType, ResampleAlg}; /// let dataset = Dataset::open("fixtures/m_3607824_se_17_1_20160620_sub.tif")?; /// let band1 = dataset.rasterband(1)?; - /// assert_eq!(band1.band_type(), u8::gdal_ordinal()); + /// assert_eq!(band1.band_type(), GdalDataType::UInt8); /// let size = 4; /// let buf = band1.read_as::((0, 0), band1.size(), (size, size), Some(ResampleAlg::Bilinear))?; /// assert_eq!(buf.size, (size, size)); @@ -476,8 +475,9 @@ impl<'a> RasterBand<'a> { } /// Returns the pixel datatype of this band. - pub fn band_type(&self) -> GDALDataType::Type { - unsafe { gdal_sys::GDALGetRasterDataType(self.c_rasterband) } + pub fn band_type(&self) -> GdalDataType { + let ordinal = unsafe { gdal_sys::GDALGetRasterDataType(self.c_rasterband) }; + ordinal.try_into().unwrap_or(GdalDataType::Unknown) } /// Returns the no-data value of this band. diff --git a/src/raster/tests.rs b/src/raster/tests.rs index 17879f31..298b189a 100644 --- a/src/raster/tests.rs +++ b/src/raster/tests.rs @@ -2,13 +2,12 @@ use crate::dataset::Dataset; use crate::metadata::Metadata; use crate::raster::rasterband::ResampleAlg; use crate::raster::{ - ByteBuffer, ColorEntry, ColorInterpretation, ColorTable, RasterCreationOption, StatisticsAll, - StatisticsMinMax, + ByteBuffer, ColorEntry, ColorInterpretation, ColorTable, GdalDataType, RasterCreationOption, + StatisticsAll, StatisticsMinMax, }; use crate::test_utils::TempFixture; use crate::vsi::unlink_mem_file; use crate::DriverManager; -use gdal_sys::GDALDataType; use std::path::Path; #[cfg(feature = "ndarray")] @@ -279,7 +278,7 @@ fn test_create_with_band_type() { assert_eq!(dataset.raster_count(), 3); assert_eq!(dataset.driver().short_name(), "MEM"); let rb = dataset.rasterband(1).unwrap(); - assert_eq!(rb.band_type(), GDALDataType::GDT_Float32) + assert_eq!(rb.band_type(), GdalDataType::Float32); } #[test] @@ -407,7 +406,7 @@ fn test_read_raster_as() { assert_eq!(rv.data, vec!(7, 7, 7, 10, 8, 12)); assert_eq!(rv.size.0, 2); assert_eq!(rv.size.1, 3); - assert_eq!(rb.band_type(), GDALDataType::GDT_Byte); + assert_eq!(rb.band_type(), GdalDataType::UInt8); } #[test] @@ -468,7 +467,7 @@ fn test_read_raster_as_array() { ]); assert_eq!(values, data); - assert_eq!(rb.band_type(), GDALDataType::GDT_Byte); + assert_eq!(rb.band_type(), GdalDataType::UInt8); } #[test] @@ -525,7 +524,7 @@ fn test_get_band_type() { let driver = DriverManager::get_driver_by_name("MEM").unwrap(); let dataset = driver.create("", 20, 10, 1).unwrap(); let rb = dataset.rasterband(1).unwrap(); - assert_eq!(rb.band_type(), GDALDataType::GDT_Byte); + assert_eq!(rb.band_type(), GdalDataType::UInt8); } #[test] @@ -817,7 +816,7 @@ fn test_create_color_table() { // Confirm we have a band without a color table. assert_eq!(dataset.raster_count(), 1); let band = dataset.rasterband(1).unwrap(); - assert_eq!(band.band_type(), GDALDataType::GDT_Byte); + assert_eq!(band.band_type(), GdalDataType::UInt8); assert!(band.color_table().is_none()); // Create a new file to put color table in diff --git a/src/raster/types.rs b/src/raster/types.rs index 10871362..c629d132 100644 --- a/src/raster/types.rs +++ b/src/raster/types.rs @@ -329,7 +329,9 @@ pub trait GdalType { /// ``` fn datatype() -> GdalDataType { // We can call `unwrap` because existence is guaranteed in this case. - Self::gdal_ordinal().try_into().expect("GdalDataType") + Self::gdal_ordinal() + .try_into() + .unwrap_or(GdalDataType::Unknown) } } From 66e87f6ffb42bf042ea3a726970e2d81413ac1ed Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Sun, 13 Nov 2022 17:32:12 -0500 Subject: [PATCH 2/6] Changelog. --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 7c571b2a..7caba087 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,7 @@ - **Breaking**: `RasterBand::band_type` returns `GdalDataType` enum instead of `GDALDataType::Type` ordinal. Fixes [#333](https://github.com/georust/gdal/issues/333) - - <> + - ## 0.14 From bb989764018ebd9586d7f4bcc874bd157564d607 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Mon, 14 Nov 2022 16:48:18 -0500 Subject: [PATCH 3/6] Hold back chrono version until deprecations can be addressed. --- .cargo/config.toml | 3 ++- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index f63d5126..9490b4f1 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,4 @@ [alias] # Run doctests, displaying compiler output -dto = "test --doc -- --show-output" \ No newline at end of file +dto = "test --doc -- --show-output" +nowarn = "clippy --all-targets -- -D warnings" \ No newline at end of file diff --git a/Cargo.toml b/Cargo.toml index fd0a8370..b0e1e974 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ libc = "0.2" geo-types = { version = "0.7" } gdal-sys = { path = "gdal-sys", version = "^0.8" } ndarray = { version = "0.15", optional = true } -chrono = { version = "0.4" } +chrono = { version = "<=0.4.22" } bitflags = "1.3" once_cell = "1.9" From 86f90e317c726dadece09b96e0aadf7329fa7222 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Tue, 15 Nov 2022 09:51:17 -0500 Subject: [PATCH 4/6] Refined chrono version range. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b0e1e974..1d4b40fb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ libc = "0.2" geo-types = { version = "0.7" } gdal-sys = { path = "gdal-sys", version = "^0.8" } ndarray = { version = "0.15", optional = true } -chrono = { version = "<=0.4.22" } +chrono = { version = ">= 0.4.0, <=0.4.22" } bitflags = "1.3" once_cell = "1.9" From 1c07337a11ce4e0ded3013e66ea3d335fa282863 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Mon, 28 Nov 2022 13:48:03 -0500 Subject: [PATCH 5/6] Update CHANGES.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Laurențiu Nicola --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 7caba087..7f8fb995 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,7 +2,7 @@ ## Unreleased -- **Breaking**: `RasterBand::band_type` returns `GdalDataType` enum instead of `GDALDataType::Type` ordinal. Fixes [#333](https://github.com/georust/gdal/issues/333) +- **Breaking**: `RasterBand::band_type` returns the `GdalDataType` enum instead of `GDALDataType::Type` ordinal. Fixes [#333](https://github.com/georust/gdal/issues/333) - From a577a93bf932b1b695f8eec43a9f907af035be53 Mon Sep 17 00:00:00 2001 From: "Simeon H.K. Fitch" Date: Mon, 28 Nov 2022 13:51:25 -0500 Subject: [PATCH 6/6] Reverted bindgen-caused warning suppressions. --- gdal-sys/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/gdal-sys/src/lib.rs b/gdal-sys/src/lib.rs index 1fb403a5..889efaba 100644 --- a/gdal-sys/src/lib.rs +++ b/gdal-sys/src/lib.rs @@ -3,11 +3,6 @@ #![allow(non_snake_case)] #![allow(clippy::upper_case_acronyms)] #![allow(rustdoc::bare_urls)] -// bindgen test code generates lots of warnings when testing -// sizes of types due to using constructs such as: -// `unsafe { &(*(::std::ptr::null::<__sbuf>()))._base as *const _ as usize }` -// This disables those warnings. -#![cfg_attr(test, allow(deref_nullptr))] include!(concat!(env!("OUT_DIR"), "/bindings.rs")); include!(concat!(env!("OUT_DIR"), "/docs_rs_helper.rs"));