allow reading dimensions from md groups #291
Conversation
| @@ -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>> { | |||
There was a problem hiding this comment.
I'm a bit groggy right now, does self here have a different lifetime from the resulting Group?
There was a problem hiding this comment.
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.
| @@ -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 { | |||
There was a problem hiding this comment.
Please try to make this unsafe block smaller :-).
There was a problem hiding this comment.
src/raster/mdarray.rs
Outdated
| let mut num_dimensions: usize = 0; | ||
| let c_dimensions = GDALGroupGetDimensions( | ||
| self.c_group, | ||
| std::ptr::addr_of_mut!(num_dimensions), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
src/raster/mdarray.rs
Outdated
| // `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")); |
There was a problem hiding this comment.
Maybe
| return Err(_last_null_pointer_err("GDALGroupGetDimensions")); | |
| return Err(_last_null_pointer_err("GDALMDArrayGetDimensions")); |
?
There was a problem hiding this comment.
|
can you resolve the conflicts? |
…-group-dimensions
|
i think this PR is good to merge. @lnicola would that be okay for you? Or are there open issues? |
|
bors r=jdroenner,lnicola |
|
Build succeeded: |
CHANGES.mdif knowledge of this change could be valuable to users.Added the wrapper for the dimension call for md groups.