Skip to content

Return None for invalid row indexes #350

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 30, 2022
Merged

Return None for invalid row indexes #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/_macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,13 @@ macro_rules! metadata_to_vector {

macro_rules! decode_metadata_row {
($T: ty, $buffer: expr) => {
match $buffer {
None => Ok(None),
Some(v) => Ok(Some(<$T as $crate::metadata::MetadataRoundtrip>::decode(
&v,
)?)),
}
<$T as $crate::metadata::MetadataRoundtrip>::decode($buffer)
};
}

macro_rules! table_row_decode_metadata {
($owner: ident, $table: ident, $pos: ident) => {
metadata_to_vector!($owner, $table, $pos).ok()?.map(|x| x)
metadata_to_vector!($owner, $table, $pos)
};
}

Expand Down
15 changes: 8 additions & 7 deletions src/edge_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ impl<'a> EdgeTable<'a> {
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
row: EdgeId,
) -> Result<Option<T>, TskitError> {
) -> Option<Result<T, TskitError>> {
let table_ref = self.table_;
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
decode_metadata_row!(T, buffer)
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
}

/// Return an iterator over rows of the table.
Expand Down Expand Up @@ -200,12 +200,13 @@ build_owned_table_type!(
/// let rowid = edges.add_row_with_metadata(0., 1., 5, 10, &metadata).unwrap();
/// assert_eq!(rowid, 0);
///
/// if let Some(decoded) = edges.metadata::<EdgeMetadata>(rowid).unwrap() {
/// assert_eq!(decoded.value, 42);
/// } else {
/// panic!("hmm...we expected some metadata!");
/// match edges.metadata::<EdgeMetadata>(rowid) {
/// // rowid is in range, decoding succeeded
/// Some(Ok(decoded)) => assert_eq!(decoded.value, 42),
/// // rowid is in range, decoding failed
/// Some(Err(e)) => panic!("error decoding metadata: {:?}", e),
/// None => panic!("row id out of range")
/// }
///
/// # }
/// ```
=> OwnedEdgeTable,
Expand Down
23 changes: 13 additions & 10 deletions src/individual_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,23 +224,23 @@ impl<'a> IndividualTable<'a> {
/// .individuals()
/// .metadata::<IndividualMetadata>(0.into())
/// {
/// Ok(metadata_option) => metadata_option,
/// Err(e) => panic!("error: {:?}", e),
/// Some(metadata_option) => metadata_option,
/// None => panic!("expected metadata"),
/// };
/// // Now, check the contents of the Option
/// match decoded_option {
/// Some(metadata) => assert_eq!(metadata.x, 1),
/// None => panic!("we expected Some(metadata)?"),
/// Ok(metadata) => assert_eq!(metadata.x, 1),
/// Err(e) => panic!("error decoding metadata: {:?}", e),
/// }
/// # }
/// ```
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
row: IndividualId,
) -> Result<Option<T>, TskitError> {
) -> Option<Result<T, TskitError>> {
let table_ref = self.table_;
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
decode_metadata_row!(T, buffer)
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
}

/// Return an iterator over rows of the table.
Expand Down Expand Up @@ -288,6 +288,7 @@ build_owned_table_type!(
/// An example with metadata.
/// This requires the cargo feature `"derive"` for `tskit`.
///
///
/// ```
/// # #[cfg(any(feature="doc", feature="derive"))] {
/// use tskit::OwnedIndividualTable;
Expand All @@ -307,10 +308,12 @@ build_owned_table_type!(
/// let rowid = individuals.add_row_with_metadata(0, None, None, &metadata).unwrap();
/// assert_eq!(rowid, 0);
///
/// if let Some(decoded) = individuals.metadata::<IndividualMetadata>(rowid).unwrap() {
/// assert_eq!(decoded.value, 42);
/// } else {
/// panic!("hmm...we expected some metadata!");
/// match individuals.metadata::<IndividualMetadata>(rowid) {
/// // rowid is in range, decoding succeeded
/// Some(Ok(decoded)) => assert_eq!(decoded.value, 42),
/// // rowid is in range, decoding failed
/// Some(Err(e)) => panic!("error decoding metadata: {:?}", e),
/// None => panic!("row id out of range")
/// }
///
/// # }
Expand Down
56 changes: 19 additions & 37 deletions src/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
//! into `Python` via the `tskit` `Python API`.

use crate::bindings::{tsk_id_t, tsk_size_t};
use crate::{SizeType, TskitError};
use crate::SizeType;
use thiserror::Error;

#[cfg(feature = "derive")]
Expand Down Expand Up @@ -257,25 +257,17 @@ pub(crate) fn char_column_to_slice<T: Sized>(
row: tsk_id_t,
num_rows: tsk_size_t,
column_length: tsk_size_t,
) -> Result<Option<&[u8]>, crate::TskitError> {
let row = match tsk_size_t::try_from(row) {
Ok(r) => r,
Err(_) => return Err(TskitError::IndexError),
) -> Option<&[u8]> {
let row = match tsk_size_t::try_from(row).ok() {
Some(r) if r < num_rows => r,
_ => return None,
};
if row >= num_rows {
return Err(crate::TskitError::IndexError {});
}
if column_length == 0 {
return Ok(None);
return None;
}
let row_isize = match isize::try_from(row) {
Ok(r) => r,
Err(_) => {
return Err(TskitError::RangeError(format!(
"could not convert u64 value {} to isize",
stringify!(row)
)))
}
let row_isize = match isize::try_from(row).ok() {
Some(x) => x,
None => return None,
};
let start = unsafe { *column_offset.offset(row_isize) };
let stop = if (row as tsk_size_t) < num_rows {
Expand All @@ -284,32 +276,22 @@ pub(crate) fn char_column_to_slice<T: Sized>(
column_length
};
if start >= stop {
return Ok(None);
return None;
}
if column_length == 0 {
return Ok(None);
return None;
}
let istart = match isize::try_from(start) {
Ok(v) => v,
Err(_) => {
return Err(TskitError::RangeError(format!(
"cauld not convert value {} to isize",
stringify!(i)
)));
}
let istart = match isize::try_from(start).ok() {
Some(v) => v,
None => return None,
};
let ustop = match usize::try_from(stop) {
Ok(v) => v,
Err(_) => {
return Err(TskitError::RangeError(format!(
"cauld not convert value {} to usize",
stringify!(i)
)));
}
let ustop = match usize::try_from(stop).ok() {
Some(v) => v,
None => return None,
};
Ok(Some(unsafe {
Some(unsafe {
std::slice::from_raw_parts(column.offset(istart) as *const u8, ustop - istart as usize)
}))
})
}

#[cfg(test)]
Expand Down
14 changes: 8 additions & 6 deletions src/migration_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,10 @@ impl<'a> MigrationTable<'a> {
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
row: MigrationId,
) -> Result<Option<T>, TskitError> {
) -> Option<Result<T, TskitError>> {
let table_ref = self.table_;
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
decode_metadata_row!(T, buffer)
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
}

/// Return an iterator over rows of the table.
Expand Down Expand Up @@ -242,10 +242,12 @@ build_owned_table_type!(
/// let rowid = migrations.add_row_with_metadata((0., 1.), 1, (0, 1), 10.3, &metadata).unwrap();
/// assert_eq!(rowid, 0);
///
/// if let Some(decoded) = migrations.metadata::<MigrationMetadata>(rowid).unwrap() {
/// assert_eq!(decoded.value, 42);
/// } else {
/// panic!("hmm...we expected some metadata!");
/// match migrations.metadata::<MigrationMetadata>(rowid) {
/// // rowid is in range, decoding succeeded
/// Some(Ok(decoded)) => assert_eq!(decoded.value, 42),
/// // rowid is in range, decoding failed
/// Some(Err(e)) => panic!("error decoding metadata: {:?}", e),
/// None => panic!("row id out of range")
/// }
///
/// # }
Expand Down
48 changes: 27 additions & 21 deletions src/mutation_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,25 @@ impl PartialEq for MutationTableRow {
}

fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option<MutationTableRow> {
let table_ref = table.table_;
Some(MutationTableRow {
id: pos.into(),
site: table.site(pos).ok()?,
node: table.node(pos).ok()?,
parent: table.parent(pos).ok()?,
time: table.time(pos).ok()?,
derived_state: table.derived_state(pos).ok()?.map(|s| s.to_vec()),
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
})
let index = ll_bindings::tsk_size_t::try_from(pos).ok()?;
match index {
i if i < table.num_rows() => {
let table_ref = table.table_;
let derived_state = table.derived_state(pos).map(|s| s.to_vec());
Some(MutationTableRow {
id: pos.into(),
site: table.site(pos).ok()?,
node: table.node(pos).ok()?,
parent: table.parent(pos).ok()?,
time: table.time(pos).ok()?,
derived_state,
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
})
}
_ => None,
}
}

pub(crate) type MutationTableRefIterator<'a> =
crate::table_iterator::TableIterator<&'a MutationTable<'a>>;
pub(crate) type MutationTableIterator<'a> = crate::table_iterator::TableIterator<MutationTable<'a>>;
Expand Down Expand Up @@ -143,10 +151,7 @@ impl<'a> MutationTable<'a> {
///
/// Will return [``IndexError``](crate::TskitError::IndexError)
/// if ``row`` is out of range.
pub fn derived_state<M: Into<MutationId>>(
&'a self,
row: M,
) -> Result<Option<&[u8]>, TskitError> {
pub fn derived_state<M: Into<MutationId>>(&'a self, row: M) -> Option<&[u8]> {
metadata::char_column_to_slice(
self,
self.table_.derived_state,
Expand All @@ -160,10 +165,10 @@ impl<'a> MutationTable<'a> {
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
row: MutationId,
) -> Result<Option<T>, TskitError> {
) -> Option<Result<T, TskitError>> {
let table_ref = self.table_;
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
decode_metadata_row!(T, buffer)
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
}

/// Return an iterator over rows of the table.
Expand Down Expand Up @@ -226,12 +231,13 @@ build_owned_table_type!(
/// let rowid = mutations.add_row_with_metadata(0, 1, 5, 10.0, None, &metadata).unwrap();
/// assert_eq!(rowid, 0);
///
/// if let Some(decoded) = mutations.metadata::<MutationMetadata>(rowid).unwrap() {
/// assert_eq!(decoded.value, 42);
/// } else {
/// panic!("hmm...we expected some metadata!");
/// match mutations.metadata::<MutationMetadata>(rowid) {
/// // rowid is in range, decoding succeeded
/// Some(Ok(decoded)) => assert_eq!(decoded.value, 42),
/// // rowid is in range, decoding failed
/// Some(Err(e)) => panic!("error decoding metadata: {:?}", e),
/// None => panic!("row id out of range")
/// }
///
/// # }
/// ```
=> OwnedMutationTable,
Expand Down
14 changes: 8 additions & 6 deletions src/node_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,10 @@ impl<'a> NodeTable<'a> {
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
row: NodeId,
) -> Result<Option<T>, TskitError> {
) -> Option<Result<T, TskitError>> {
let table_ref = self.table_;
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
decode_metadata_row!(T, buffer)
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
}

/// Return an iterator over rows of the table.
Expand Down Expand Up @@ -285,10 +285,12 @@ build_owned_table_type!(
/// let rowid = nodes.add_row_with_metadata(0, 1., -1, -1, &metadata).unwrap();
/// assert_eq!(rowid, 0);
///
/// if let Some(decoded) = nodes.metadata::<NodeMetadata>(rowid).unwrap() {
/// assert_eq!(decoded.value, 42);
/// } else {
/// panic!("hmm...we expected some metadata!");
/// match nodes.metadata::<NodeMetadata>(rowid) {
/// // rowid is in range, decoding succeeded
/// Some(Ok(decoded)) => assert_eq!(decoded.value, 42),
/// // rowid is in range, decoding failed
/// Some(Err(e)) => panic!("error decoding metadata: {:?}", e),
/// None => panic!("row id out of range")
/// }
///
/// # }
Expand Down
31 changes: 20 additions & 11 deletions src/population_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,18 @@ impl PartialEq for PopulationTableRow {

fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option<PopulationTableRow> {
let table_ref = table.table_;
Some(PopulationTableRow {
id: pos.into(),
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
})
let index = ll_bindings::tsk_size_t::try_from(pos).ok()?;

match index {
i if i < table.num_rows() => {
let metadata = table_row_decode_metadata!(table, table_ref, pos).map(|s| s.to_vec());
Some(PopulationTableRow {
id: pos.into(),
metadata,
})
}
_ => None,
}
}

pub(crate) type PopulationTableRefIterator<'a> =
Expand Down Expand Up @@ -75,10 +83,10 @@ impl<'a> PopulationTable<'a> {
pub fn metadata<T: metadata::MetadataRoundtrip>(
&'a self,
row: PopulationId,
) -> Result<Option<T>, TskitError> {
) -> Option<Result<T, TskitError>> {
let table_ref = self.table_;
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
decode_metadata_row!(T, buffer)
Some(decode_metadata_row!(T, buffer).map_err(TskitError::from))
}

/// Return an iterator over rows of the table.
Expand Down Expand Up @@ -144,12 +152,13 @@ build_owned_table_type!(
/// let rowid = populations.add_row_with_metadata(&metadata).unwrap();
/// assert_eq!(rowid, 0);
///
/// if let Some(decoded) = populations.metadata::<PopulationMetadata>(rowid).unwrap() {
/// assert_eq!(&decoded.name, "YRB");
/// } else {
/// panic!("hmm...we expected some metadata!");
/// match populations.metadata::<PopulationMetadata>(rowid) {
/// // rowid is in range, decoding succeeded
/// Some(Ok(decoded)) => assert_eq!(&decoded.name, "YRB"),
/// // rowid is in range, decoding failed
/// Some(Err(e)) => panic!("error decoding metadata: {:?}", e),
/// None => panic!("row id out of range")
/// }
///
/// # }
/// ```
=> OwnedPopulationTable,
Expand Down
Loading