Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow reading dimensions from md groups #291

Merged
merged 11 commits into from
Sep 2, 2022
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

- Add prebuild bindings for GDAL 3.5

- <https://github.com/georust/gdal/pull/277>

- **Breaking**: Add `gdal::vector::OwnedLayer`, `gdal::vector::LayerAccess` and `gdal::vector::layer::OwnedFeatureIterator`. This requires importing `gdal::vector::LayerAccess` for using most vector layer methods.
Expand Down Expand Up @@ -71,6 +72,10 @@

- <https://github.com/georust/gdal/pull/284>

- Allow reading `Dimension`s from `Group`s in multimensional `Dataset`s.

- <https://github.com/georust/gdal/pull/291>

## 0.12

- Bump Rust edition to 2021
Expand Down
96 changes: 82 additions & 14 deletions src/raster/mdarray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use gdal_sys::{
GDALAttributeRelease, GDALDataType, GDALDimensionGetIndexingVariable, GDALDimensionGetName,
GDALDimensionGetSize, GDALDimensionHS, GDALDimensionRelease, GDALExtendedDataTypeClass,
GDALExtendedDataTypeGetClass, GDALExtendedDataTypeGetNumericDataType, GDALExtendedDataTypeH,
GDALExtendedDataTypeRelease, GDALGroupGetAttribute, GDALGroupGetGroupNames,
GDALGroupGetMDArrayNames, GDALGroupGetName, GDALGroupH, GDALGroupOpenGroup,
GDALGroupOpenMDArray, GDALGroupRelease, GDALMDArrayGetAttribute, GDALMDArrayGetDataType,
GDALMDArrayGetDimensionCount, GDALMDArrayGetDimensions, GDALMDArrayGetNoDataValueAsDouble,
GDALMDArrayGetSpatialRef, GDALMDArrayGetTotalElementsCount, GDALMDArrayGetUnit, GDALMDArrayH,
GDALMDArrayRelease, OSRDestroySpatialReference, VSIFree,
GDALExtendedDataTypeRelease, GDALGroupGetAttribute, GDALGroupGetDimensions,
GDALGroupGetGroupNames, GDALGroupGetMDArrayNames, GDALGroupGetName, GDALGroupH,
GDALGroupOpenGroup, GDALGroupOpenMDArray, GDALGroupRelease, GDALMDArrayGetAttribute,
GDALMDArrayGetDataType, GDALMDArrayGetDimensionCount, GDALMDArrayGetDimensions,
GDALMDArrayGetNoDataValueAsDouble, GDALMDArrayGetSpatialRef, GDALMDArrayGetTotalElementsCount,
GDALMDArrayGetUnit, GDALMDArrayH, GDALMDArrayRelease, OSRDestroySpatialReference, VSIFree,
};
use libc::c_void;
use std::ffi::CString;
Expand All @@ -37,11 +37,17 @@ pub struct MDArray<'a> {
}

#[derive(Debug)]
enum GroupOrDimension<'a> {
pub enum GroupOrDimension<'a> {
Group { _group: &'a Group<'a> },
Dimension { _dimension: &'a Dimension<'a> },
}

#[derive(Debug)]
pub enum GroupOrArray<'a> {
Group { _group: &'a Group<'a> },
MDArray { _md_array: &'a MDArray<'a> },
}

impl Drop for MDArray<'_> {
fn drop(&mut self) {
unsafe {
Expand Down Expand Up @@ -90,16 +96,21 @@ impl<'a> MDArray<'a> {
let c_dimensions =
GDALMDArrayGetDimensions(self.c_mdarray, std::ptr::addr_of_mut!(num_dimensions));

if c_dimensions.is_null() {
return Err(_last_null_pointer_err("GDALMDArrayGetDimensions"));
// `num_dimensions` is `0`, we can safely return an empty vector
// `GDALMDArrayGetDimensions` does not state that errors can occur
if num_dimensions > 0 && c_dimensions.is_null() {
return Err(_last_null_pointer_err("GDALGroupGetDimensions"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe

Suggested change
return Err(_last_null_pointer_err("GDALGroupGetDimensions"));
return Err(_last_null_pointer_err("GDALMDArrayGetDimensions"));

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

let dimensions_ref = std::slice::from_raw_parts_mut(c_dimensions, num_dimensions);

let mut dimensions: Vec<Dimension> = Vec::with_capacity(num_dimensions);

for c_dimension in dimensions_ref {
let dimension = Dimension::from_c_dimension(self, *c_dimension);
let dimension = Dimension::from_c_dimension(
GroupOrArray::MDArray { _md_array: self },
*c_dimension,
);
dimensions.push(dimension);
}

Expand Down Expand Up @@ -413,7 +424,7 @@ impl<'a> Group<'a> {
}
}

pub fn open_group(&self, name: &str, options: CslStringList) -> Result<Group> {
pub fn open_group(&'_ self, name: &str, options: CslStringList) -> Result<Group<'a>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit groggy right now, does self here have a different lifetime from the resulting Group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I prevented Group from having the lifetime of self. It needs the lifetime of the dataset.
I tested that in the changed test where I dropped a super-group.

let name = CString::new(name)?;

unsafe {
Expand All @@ -440,13 +451,45 @@ impl<'a> Group<'a> {
Ok(Attribute::from_c_attribute(c_attribute))
}
}

pub fn dimensions(&self, options: CslStringList) -> Result<Vec<Dimension>> {
unsafe {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try to make this unsafe block smaller :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let mut num_dimensions: usize = 0;
let c_dimensions = GDALGroupGetDimensions(
self.c_group,
std::ptr::addr_of_mut!(num_dimensions),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not necessarily against it, but I'd expect to see addr_of_mut! used with uninitialized variables, so it's a bit confusing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.as_ptr(),
);

// `num_dimensions` is `0`, we can safely return an empty vector
// `GDALGroupGetDimensions` does not state that errors can occur
if num_dimensions > 0 && c_dimensions.is_null() {
return Err(_last_null_pointer_err("GDALGroupGetDimensions"));
}

let dimensions_ref = std::slice::from_raw_parts_mut(c_dimensions, num_dimensions);

let mut dimensions: Vec<Dimension> = Vec::with_capacity(num_dimensions);

for c_dimension in dimensions_ref {
let dimension =
Dimension::from_c_dimension(GroupOrArray::Group { _group: self }, *c_dimension);
dimensions.push(dimension);
}

// only free the array, not the dimensions themselves
VSIFree(c_dimensions as *mut c_void);

Ok(dimensions)
}
}
}

/// A `GDALDimension` with name and size
#[derive(Debug)]
pub struct Dimension<'a> {
c_dimension: *mut GDALDimensionHS,
_md_array: &'a MDArray<'a>,
_parent: GroupOrArray<'a>,
}

impl Drop for Dimension<'_> {
Expand All @@ -462,10 +505,13 @@ impl<'a> Dimension<'a> {
///
/// # Safety
/// This method operates on a raw C pointer
pub fn from_c_dimension(_md_array: &'a MDArray<'a>, c_dimension: *mut GDALDimensionHS) -> Self {
pub unsafe fn from_c_dimension(
_parent: GroupOrArray<'a>,
c_dimension: *mut GDALDimensionHS,
) -> Self {
Self {
c_dimension,
_md_array,
_parent,
}
}
pub fn size(&self) -> usize {
Expand Down Expand Up @@ -708,6 +754,17 @@ mod tests {
};
let dataset = Dataset::open_ex("fixtures/byte_no_cf.nc", dataset_options).unwrap();
let root_group = dataset.root_group().unwrap();

// group dimensions
let group_dimensions = root_group.dimensions(CslStringList::new()).unwrap();
let group_dimensions_names: Vec<String> = group_dimensions
.into_iter()
.map(|dimensions| dimensions.name())
.collect();
assert_eq!(group_dimensions_names, vec!["x", "y"]);

// array dimensions

let array_name = "Band1".to_string();
let options = CslStringList::new(); //Driver specific options determining how the array should be opened. Pass nullptr for default behavior.
let md_array = root_group.open_md_array(&array_name, options).unwrap();
Expand All @@ -718,6 +775,7 @@ mod tests {
}
assert_eq!(dimension_names, vec!["y".to_string(), "x".to_string()])
}

#[test]
fn test_dimension_size() {
let dataset_options = DatasetOptions {
Expand Down Expand Up @@ -796,6 +854,7 @@ mod tests {
let dataset = Dataset::open_ex("fixtures/byte_no_cf.nc", dataset_options).unwrap();

let root_group = dataset.root_group().unwrap();

let md_array = root_group
.open_md_array("Band1", CslStringList::new())
.unwrap();
Expand Down Expand Up @@ -866,6 +925,12 @@ mod tests {
let group_science = root_group
.open_group("science", CslStringList::new())
.unwrap();

assert!(group_science
.dimensions(Default::default())
.unwrap()
.is_empty());

let group_grids = group_science
.open_group("grids", CslStringList::new())
.unwrap();
Expand Down Expand Up @@ -914,6 +979,9 @@ mod tests {
let group_grids = group_science
.open_group("grids", CslStringList::new())
.unwrap();

drop(group_science); // check that `Group`s do not borrow each other

let group_data = group_grids
.open_group("data", CslStringList::new())
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/raster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod types;
mod warp;

#[cfg(all(major_ge_3, minor_ge_1))]
pub use mdarray::{ExtendedDataType, Group, MDArray};
pub use mdarray::{Attribute, Dimension, ExtendedDataType, Group, MDArray};
pub use rasterband::{
Buffer, ByteBuffer, CmykEntry, ColorEntry, ColorInterpretation, ColorTable, GrayEntry,
HlsEntry, PaletteInterpretation, RasterBand, ResampleAlg, RgbaEntry,
Expand Down