refactor row access

examples/forward_simulation.rs

@@ -373,7 +373,7 @@ fn update_bookmark(
         for a in alive {
             for node in [a.node0, a.node1] {
                 match nodes.time(node) {
-                    Ok(time) => {
+                    Some(time) => {
                         most_recent_birth_time = if time < most_recent_birth_time {
                             time.into()
                         } else {
@@ -385,7 +385,7 @@ fn update_bookmark(
                             most_ancient_birth_time
                         };
                     }
-                    Err(e) => return Err(e),
+                    None => return Err(tskit::TskitError::IndexError {}),
                 }
             }
         }

src/_macros.rs

@@ -30,27 +30,33 @@ macro_rules! panic_on_tskit_error {
 macro_rules! unsafe_tsk_column_access {
     ($i: expr, $lo: expr, $hi: expr, $array: expr) => {{
         if $i < $lo || ($i as $crate::tsk_size_t) >= $hi {
-            Err($crate::error::TskitError::IndexError {})
+            None
         } else {
-            Ok(unsafe { *$array.offset($i as isize) })
+            Some(unsafe { *$array.offset($i as isize) })
         }
     }};
     ($i: expr, $lo: expr, $hi: expr, $array: expr, $output_id_type: expr) => {{
         if $i < $lo || ($i as $crate::tsk_size_t) >= $hi {
-            Err($crate::error::TskitError::IndexError {})
+            None
         } else {
-            Ok($output_id_type(unsafe { *$array.offset($i as isize) }))
+            Some($output_id_type(unsafe { *$array.offset($i as isize) }))
         }
     }};
 }
 
+macro_rules! unsafe_tsk_column_access_and_map_into {
+    ($i: expr, $lo: expr, $hi: expr, $array: expr) => {{
+        unsafe_tsk_column_access!($i, $lo, $hi, $array).map(|v| v.into())
+    }};
+}
+
 macro_rules! unsafe_tsk_ragged_column_access {
     ($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr) => {{
-        let i = $crate::SizeType::try_from($i)?;
+        let i = $crate::SizeType::try_from($i).ok()?;
         if $i < $lo || i >= $hi {
-            Err(TskitError::IndexError {})
+            None
         } else if $offset_array_len == 0 {
-            Ok(None)
+            None
         } else {
             let start = unsafe { *$offset_array.offset($i as isize) };
             let stop = if i < $hi {
@@ -59,23 +65,23 @@ macro_rules! unsafe_tsk_ragged_column_access {
                 $offset_array_len as tsk_size_t
             };
             if start == stop {
-                Ok(None)
+                None
             } else {
                 let mut buffer = vec![];
                 for i in start..stop {
                     buffer.push(unsafe { *$array.offset(i as isize) });
                 }
-                Ok(Some(buffer))
+                Some(buffer)
             }
         }
     }};
 
     ($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr, $output_id_type: ty) => {{
-        let i = $crate::SizeType::try_from($i)?;
+        let i = $crate::SizeType::try_from($i).ok()?;
         if $i < $lo || i >= $hi {
-            Err(TskitError::IndexError {})
+            None
         } else if $offset_array_len == 0 {
-            Ok(None)
+            None
         } else {
             let start = unsafe { *$offset_array.offset($i as isize) };
             let stop = if i < $hi {
@@ -84,14 +90,14 @@ macro_rules! unsafe_tsk_ragged_column_access {
                 $offset_array_len as tsk_size_t
             };
             if start == stop {
-                Ok(None)
+                None
             } else {
-                Ok(Some(unsafe {
+                Some(unsafe {
                     std::slice::from_raw_parts(
                         $array.offset(start as isize) as *const $output_id_type,
                         stop as usize - start as usize,
                     )
-                }))
+                })
             }
         }
     }};

src/edge_table.rs

@@ -30,10 +30,10 @@ fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow>
     let table_ref = table.table_;
     Some(EdgeTableRow {
         id: pos.into(),
-        left: table.left(pos).ok()?,
-        right: table.right(pos).ok()?,
-        parent: table.parent(pos).ok()?,
-        child: table.child(pos).ok()?,
+        left: table.left(pos)?,
+        right: table.right(pos)?,
+        parent: table.parent(pos)?,
+        child: table.child(pos)?,
         metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
     })
 }
@@ -83,48 +83,42 @@ impl<'a> EdgeTable<'a> {
 
     /// Return the ``parent`` value from row ``row`` of the table.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// Will return [``IndexError``](crate::TskitError::IndexError)
-    /// if ``row`` is out of range.
-    pub fn parent<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<NodeId, TskitError> {
+    /// * `Some(parent)` if `u` is valid.
+    /// * `None` otherwise.
+    pub fn parent<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<NodeId> {
         unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.parent, NodeId)
     }
 
     /// Return the ``child`` value from row ``row`` of the table.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// Will return [``IndexError``](crate::TskitError::IndexError)
-    /// if ``row`` is out of range.
-    pub fn child<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<NodeId, TskitError> {
+    /// * `Some(child)` if `u` is valid.
+    /// * `None` otherwise.
+    pub fn child<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<NodeId> {
         unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.child, NodeId)
     }
 
     /// Return the ``left`` value from row ``row`` of the table.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// Will return [``IndexError``](crate::TskitError::IndexError)
-    /// if ``row`` is out of range.
-    pub fn left<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<Position, TskitError> {
-        match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left) {
-            Ok(p) => Ok(p.into()),
-            Err(e) => Err(e),
-        }
+    /// * `Some(position)` if `u` is valid.
+    /// * `None` otherwise.
+    pub fn left<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<Position> {
+        unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left, Position)
     }
 
     /// Return the ``right`` value from row ``row`` of the table.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// Will return [``IndexError``](crate::TskitError::IndexError)
-    /// if ``row`` is out of range.
-    pub fn right<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<Position, TskitError> {
-        match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.right) {
-            Ok(p) => Ok(p.into()),
-            Err(e) => Err(e),
-        }
+    /// * `Some(position)` if `u` is valid.
+    /// * `None` otherwise.
+    pub fn right<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<Position> {
+        unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.right)
     }
 
     pub fn metadata<T: metadata::MetadataRoundtrip>(

src/individual_table.rs

@@ -55,9 +55,9 @@ fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<I
     let table_ref = table.table_;
     Some(IndividualTableRow {
         id: pos.into(),
-        flags: table.flags(pos).ok()?,
-        location: table.location(pos).ok()?.map(|s| s.to_vec()),
-        parents: table.parents(pos).ok()?.map(|s| s.to_vec()),
+        flags: table.flags(pos)?,
+        location: table.location(pos).map(|s| s.to_vec()),
+        parents: table.parents(pos).map(|s| s.to_vec()),
         metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
     })
 }
@@ -101,28 +101,21 @@ impl<'a> IndividualTable<'a> {
 
     /// Return the flags for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn flags<I: Into<IndividualId> + Copy>(
-        &self,
-        row: I,
-    ) -> Result<IndividualFlags, TskitError> {
-        match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.flags) {
-            Ok(f) => Ok(IndividualFlags::from(f)),
-            Err(e) => Err(e),
-        }
+    /// * `Some(flags)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn flags<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<IndividualFlags> {
+        unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.flags)
     }
 
     /// Return the locations for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn location<I: Into<IndividualId> + Copy>(
-        &self,
-        row: I,
-    ) -> Result<Option<&[Location]>, TskitError> {
+    /// * `Some(location)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn location<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<&[Location]> {
         unsafe_tsk_ragged_column_access!(
             row.into().0,
             0,
@@ -136,13 +129,11 @@ impl<'a> IndividualTable<'a> {
 
     /// Return the parents for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn parents<I: Into<IndividualId> + Copy>(
-        &self,
-        row: I,
-    ) -> Result<Option<&[IndividualId]>, TskitError> {
+    /// * `Some(parents)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn parents<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<&[IndividualId]> {
         unsafe_tsk_ragged_column_access!(
             row.into().0,
             0,

src/migration_table.rs

@@ -36,12 +36,12 @@ fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<Mig
     let table_ref = table.table_;
     Some(MigrationTableRow {
         id: pos.into(),
-        left: table.left(pos).ok()?,
-        right: table.right(pos).ok()?,
-        node: table.node(pos).ok()?,
-        source: table.source(pos).ok()?,
-        dest: table.dest(pos).ok()?,
-        time: table.time(pos).ok()?,
+        left: table.left(pos)?,
+        right: table.right(pos)?,
+        node: table.node(pos)?,
+        source: table.source(pos)?,
+        dest: table.dest(pos)?,
+        time: table.time(pos)?,
         metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
     })
 }
@@ -92,46 +92,41 @@ impl<'a> MigrationTable<'a> {
 
     /// Return the left coordinate for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn left<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<Position, TskitError> {
-        match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left) {
-            Ok(p) => Ok(p.into()),
-            Err(e) => Err(e),
-        }
+    /// * `Some(position)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn left<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<Position> {
+        unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.left)
     }
 
     /// Return the right coordinate for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn right<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<Position, TskitError> {
-        match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.right) {
-            Ok(p) => Ok(p.into()),
-            Err(e) => Err(e),
-        }
+    /// * `Some(positions)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn right<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<Position> {
+        unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.right)
     }
 
     /// Return the node for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn node<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<NodeId, TskitError> {
+    /// * `Some(node)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn node<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<NodeId> {
         unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.node, NodeId)
     }
 
     /// Return the source population for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn source<M: Into<MigrationId> + Copy>(
-        &'a self,
-        row: M,
-    ) -> Result<PopulationId, TskitError> {
+    /// * `Some(population)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn source<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<PopulationId> {
         unsafe_tsk_column_access!(
             row.into().0,
             0,
@@ -143,10 +138,11 @@ impl<'a> MigrationTable<'a> {
 
     /// Return the destination population for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn dest<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<PopulationId, TskitError> {
+    /// * `Some(population)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn dest<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<PopulationId> {
         unsafe_tsk_column_access!(
             row.into().0,
             0,
@@ -158,21 +154,21 @@ impl<'a> MigrationTable<'a> {
 
     /// Return the time of the migration event for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
-    pub fn time<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<Time, TskitError> {
-        match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.time) {
-            Ok(t) => Ok(t.into()),
-            Err(e) => Err(e),
-        }
+    /// * `Some(time)` if `row` is valid.
+    /// * `None` otherwise.
+    pub fn time<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<Time> {
+        unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.time)
     }
 
     /// Return the metadata for a given row.
     ///
-    /// # Errors
+    /// # Returns
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
+    /// * `Some(Ok(metadata))` if `row` is valid and decoding succeeded
+    /// * `Some(Err(_))` if `row` is valid and decoding failed.
+    /// * `None` if `row` is not valid.
     pub fn metadata<T: metadata::MetadataRoundtrip>(
         &'a self,
         row: MigrationId,