Skip to content

Commit

Permalink
Update to 0.11 (#20)
Browse files Browse the repository at this point in the history 
- add read/write API with static types
- use generics in `CodesHandle`
- use `Vec<u8>` instead of `Bytes` for in-memory files
- introduce better abstraction for different kinds of `CodesHandle`
- remove `fclose()` from `CodesHandle destructor
- remove redundant ecCodes destructors calls
- ensure correct destruction order of objects
- add more complex tests of destructors
- made minor improvements to several tests and docs
  • Loading branch information
@Quba1
Quba1 authored Jul 27, 2024
1 parent b21db71 commit 3f5b41e
Show file tree
Hide file tree
Showing 21 changed files with 1,046 additions and 689 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/rust.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ jobs:
cargo clean
- name: Test with cargo
run: |
cargo test --no-default-features
cargo test --features "message_ndarray"
cargo test --features "experimental_index"
cargo test --features "experimental_index, message_ndarray"
cargo clean
- name: Benchmark with criterion
Expand Down Expand Up @@ -79,6 +82,9 @@ jobs:
cargo clean
- name: Test with cargo
run: |
cargo test --no-default-features
cargo test --features "message_ndarray"
cargo test --features "experimental_index"
cargo test --features "experimental_index, message_ndarray"
cargo clean
- name: Benchmark with criterion
Expand Down
3 changes: 1 addition & 2 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "eccodes"
description = "Unofficial high-level Rust bindings of the latest ecCodes release"
repository = "https://github.com/ScaleWeather/eccodes"
version = "0.10.4"
version = "0.11.0"
readme = "README.md"
authors = ["Jakub Lewandowski <[email protected]>"]
keywords = ["eccodes", "grib", "bufr", "meteorology", "weather"]
Expand All @@ -22,7 +22,6 @@ rust-version = "1.70.0"
eccodes-sys = { version = "0.5.2", default-features = false }
libc = { version = "0.2", default-features = false }
thiserror = { version = "1.0", default-features = false }
bytes = { version = "1.6", default-features = false }
log = { version = "0.4", default-features = false }
errno = { version = "0.3", default-features = false }
num-derive = { version = "0.4", default-features = false }
Expand Down
19 changes: 11 additions & 8 deletions benches/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,36 +14,39 @@ pub fn key_reading(c: &mut Criterion) {
let msg = handle.next().unwrap().unwrap();

c.bench_function("long reading", |b| {
b.iter(|| msg.read_key(black_box("dataDate")).unwrap())
b.iter(|| msg.read_key_dynamic(black_box("dataDate")).unwrap())
});

c.bench_function("double reading", |b| {
b.iter(|| {
msg.read_key(black_box("jDirectionIncrementInDegrees"))
msg.read_key_dynamic(black_box("jDirectionIncrementInDegrees"))
.unwrap()
})
});

c.bench_function("double array reading", |b| {
b.iter(|| msg.read_key(black_box("values")).unwrap())
b.iter(|| msg.read_key_dynamic(black_box("values")).unwrap())
});

c.bench_function("string reading", |b| {
b.iter(|| msg.read_key(black_box("name")).unwrap())
b.iter(|| msg.read_key_dynamic(black_box("name")).unwrap())
});

c.bench_function("bytes reading", |b| {
b.iter(|| msg.read_key(black_box("section1Padding")).unwrap())
b.iter(|| msg.read_key_dynamic(black_box("section1Padding")).unwrap())
});

c.bench_function("missing nul-byte termination reading", |b| {
b.iter(|| msg.read_key(black_box("experimentVersionNumber")).unwrap())
b.iter(|| {
msg.read_key_dynamic(black_box("experimentVersionNumber"))
.unwrap()
})
});

c.bench_function("problematic key reading", |b| {
b.iter(|| {
msg.read_key(black_box("zero"))
.unwrap_or_else(|_| msg.read_key(black_box("zeros")).unwrap())
msg.read_key_dynamic(black_box("zero"))
.unwrap_or_else(|_| msg.read_key_dynamic(black_box("zeros")).unwrap())
})
});
}
Expand Down
108 changes: 27 additions & 81 deletions src/codes_handle/iterator.rs
View file
Original file line number Diff line number Diff line change
@@ -1,97 +1,42 @@
use std::ptr;

use crate::{codes_handle::HandleGenerator, errors::CodesError, CodesHandle, KeyedMessage};
use fallible_streaming_iterator::FallibleStreamingIterator;

use crate::{
errors::CodesError,
intermediate_bindings::{codes_handle_delete, codes_handle_new_from_file},
CodesHandle, KeyedMessage,
};
#[cfg(feature = "experimental_index")]
use crate::{intermediate_bindings::codes_handle_new_from_index, CodesIndex};

use super::GribFile;
use std::fmt::Debug;

/// # Errors
///
/// The `advance()` and `next()` methods will return [`CodesInternal`](crate::errors::CodesInternal)
/// when internal ecCodes function returns non-zero code.
impl FallibleStreamingIterator for CodesHandle<GribFile> {
impl<S: HandleGenerator + Debug> FallibleStreamingIterator for CodesHandle<S> {
type Item = KeyedMessage;

type Error = CodesError;

fn advance(&mut self) -> Result<(), Self::Error> {
unsafe {
codes_handle_delete(self.unsafe_message.message_handle)?;
}

// nullify message handle so that destructor is harmless
// it might be excessive but it follows the correct pattern
self.unsafe_message.message_handle = ptr::null_mut();

let new_eccodes_handle =
unsafe { codes_handle_new_from_file(self.source.pointer, self.product_kind)? };

self.unsafe_message = KeyedMessage {
message_handle: new_eccodes_handle,
};
// destructor of KeyedMessage calls ecCodes

Ok(())
}
let new_eccodes_handle = self.source.gen_codes_handle()?;

fn get(&self) -> Option<&Self::Item> {
if self.unsafe_message.message_handle.is_null() {
self.current_message = if new_eccodes_handle.is_null() {
None
} else {
Some(&self.unsafe_message)
}
}
}

#[cfg(feature = "experimental_index")]
#[cfg_attr(docsrs, doc(cfg(feature = "experimental_index")))]
/// # Errors
///
/// The `advance()` and `next()` methods will return [`CodesInternal`](crate::errors::CodesInternal)
/// when internal ecCodes function returns non-zero code.
impl FallibleStreamingIterator for CodesHandle<CodesIndex> {
type Item = KeyedMessage;

type Error = CodesError;

fn advance(&mut self) -> Result<(), Self::Error> {
unsafe {
codes_handle_delete(self.unsafe_message.message_handle)?;
}

// nullify message handle so that destructor is harmless
// it might be excessive but it follows the correct pattern
self.unsafe_message.message_handle = ptr::null_mut();

let new_eccodes_handle = unsafe { codes_handle_new_from_index(self.source.pointer)? };

self.unsafe_message = KeyedMessage {
message_handle: new_eccodes_handle,
Some(KeyedMessage {
message_handle: new_eccodes_handle,
})
};

Ok(())
}

fn get(&self) -> Option<&Self::Item> {
if self.unsafe_message.message_handle.is_null() {
None
} else {
Some(&self.unsafe_message)
}
self.current_message.as_ref()
}
}

#[cfg(test)]
mod tests {
use crate::{
codes_handle::{CodesHandle, ProductKind},
KeyType,
DynamicKeyType,
};
use anyhow::{Context, Ok, Result};
use fallible_streaming_iterator::FallibleStreamingIterator;
Expand All @@ -104,17 +49,17 @@ mod tests {
let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;

let msg1 = handle.next()?.context("Message not some")?;
let key1 = msg1.read_key("typeOfLevel")?;
let key1 = msg1.read_key_dynamic("typeOfLevel")?;

let msg2 = handle.next()?.context("Message not some")?;
let key2 = msg2.read_key("typeOfLevel")?;
let key2 = msg2.read_key_dynamic("typeOfLevel")?;

let msg3 = handle.next()?.context("Message not some")?;
let key3 = msg3.read_key("typeOfLevel")?;
let key3 = msg3.read_key_dynamic("typeOfLevel")?;

assert_eq!(key1.value, KeyType::Str("isobaricInhPa".to_string()));
assert_eq!(key2.value, KeyType::Str("isobaricInhPa".to_string()));
assert_eq!(key3.value, KeyType::Str("isobaricInhPa".to_string()));
assert_eq!(key1, DynamicKeyType::Str("isobaricInhPa".to_string()));
assert_eq!(key2, DynamicKeyType::Str("isobaricInhPa".to_string()));
assert_eq!(key3, DynamicKeyType::Str("isobaricInhPa".to_string()));

Ok(())
}
Expand All @@ -127,10 +72,10 @@ mod tests {
let mut handle = CodesHandle::new_from_file(file_path, product_kind)?;

while let Some(msg) = handle.next()? {
let key = msg.read_key("shortName")?;
let key = msg.read_key_dynamic("shortName")?;

match key.value {
KeyType::Str(_) => {}
match key {
DynamicKeyType::Str(_) => {}
_ => panic!("Incorrect variant of string key"),
}
}
Expand All @@ -151,9 +96,9 @@ mod tests {
}

for msg in handle_collected {
let key = msg.read_key("name")?;
match key.value {
KeyType::Str(_) => {}
let key: DynamicKeyType = msg.read_key_dynamic("name")?;
match key {
DynamicKeyType::Str(_) => {}
_ => panic!("Incorrect variant of string key"),
}
}
Expand Down Expand Up @@ -207,8 +152,9 @@ mod tests {
let mut level = vec![];

while let Some(msg) = handle.next()? {
if msg.read_key("shortName")?.value == KeyType::Str("msl".to_string())
&& msg.read_key("typeOfLevel")?.value == KeyType::Str("surface".to_string())
if msg.read_key_dynamic("shortName")? == DynamicKeyType::Str("msl".to_string())
&& msg.read_key_dynamic("typeOfLevel")?
== DynamicKeyType::Str("surface".to_string())
{
level.push(msg.try_clone()?);
}
Expand All @@ -218,7 +164,7 @@ mod tests {
// Find nearest modifies internal KeyedMessage fields so we need mutable reference
let level = &level[0];

println!("{:?}", level.read_key("shortName"));
println!("{:?}", level.read_key_dynamic("shortName"));

// Get the four nearest gridpoints of Reykjavik
let nearest_gridpoints = level.codes_nearest()?.find_nearest(64.13, -21.89)?;
Expand Down
Loading

0 comments on commit 3f5b41e

Please sign in to comment.