streamline row building

src/_macros.rs

@@ -153,9 +153,7 @@ macro_rules! decode_metadata_row {
 
 macro_rules! table_row_decode_metadata {
     ($owner: ident, $table: ident, $pos: ident) => {
-        metadata_to_vector!($owner, $table, $pos)
-            .unwrap()
-            .map(|x| x)
+        metadata_to_vector!($owner, $table, $pos).ok()?.map(|x| x)
     };
 }
 
@@ -184,17 +182,10 @@ macro_rules! err_if_not_tracking_samples {
 // This macro assumes that table row access helper
 // functions have a standard interface.
 // Here, we convert the None type to an Error,
-// as it applies $row is out of range.
+// as it implies $row is out of range.
 macro_rules! table_row_access {
     ($row: expr, $table: expr, $row_fn: ident) => {
-        if $row < 0 {
-            Err(TskitError::IndexError)
-        } else {
-            match $row_fn($table, $row) {
-                Some(x) => Ok(x),
-                None => Err(TskitError::IndexError),
-            }
-        }
+        $row_fn($table, $row).ok_or_else(|| TskitError::IndexError {})
     };
 }
 

src/edge_table.rs

@@ -27,24 +27,15 @@ impl PartialEq for EdgeTableRow {
 }
 
 fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            let table_ref = table.table_;
-            let rv = EdgeTableRow {
-                id: pos.into(),
-                left: table.left(pos).unwrap(),
-                right: table.right(pos).unwrap(),
-                parent: table.parent(pos).unwrap(),
-                child: table.child(pos).unwrap(),
-                metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
-            };
-            Some(rv)
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    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()?,
+        metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
+    })
 }
 
 pub(crate) type EdgeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a EdgeTable<'a>>;

src/migration_table.rs

@@ -33,25 +33,17 @@ impl PartialEq for MigrationTableRow {
 }
 
 fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<MigrationTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            let table_ref = table.table_;
-            Some(MigrationTableRow {
-                id: pos.into(),
-                left: table.left(pos).unwrap(),
-                right: table.right(pos).unwrap(),
-                node: table.node(pos).unwrap(),
-                source: table.source(pos).unwrap(),
-                dest: table.dest(pos).unwrap(),
-                time: table.time(pos).unwrap(),
-                metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
-            })
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    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()?,
+        metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
+    })
 }
 
 pub(crate) type MigrationTableRefIterator<'a> =

src/mutation_table.rs

@@ -30,25 +30,16 @@ impl PartialEq for MutationTableRow {
 }
 
 fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option<MutationTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            let table_ref = table.table_;
-            let rv = MutationTableRow {
-                id: pos.into(),
-                site: table.site(pos).unwrap(),
-                node: table.node(pos).unwrap(),
-                parent: table.parent(pos).unwrap(),
-                time: table.time(pos).unwrap(),
-                derived_state: table.derived_state(pos).unwrap().map(|s| s.to_vec()),
-                metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
-            };
-            Some(rv)
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    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()),
+    })
 }
 pub(crate) type MutationTableRefIterator<'a> =
     crate::table_iterator::TableIterator<&'a MutationTable<'a>>;

src/node_table.rs

@@ -29,23 +29,15 @@ impl PartialEq for NodeTableRow {
 }
 
 fn make_node_table_row(table: &NodeTable, pos: tsk_id_t) -> Option<NodeTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            let table_ref = table.table_;
-            Some(NodeTableRow {
-                id: pos.into(),
-                time: table.time(pos).unwrap(),
-                flags: table.flags(pos).unwrap(),
-                population: table.population(pos).unwrap(),
-                individual: table.individual(pos).unwrap(),
-                metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
-            })
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    let table_ref = table.table_;
+    Some(NodeTableRow {
+        id: pos.into(),
+        time: table.time(pos).ok()?,
+        flags: table.flags(pos).ok()?,
+        population: table.population(pos).ok()?,
+        individual: table.individual(pos).ok()?,
+        metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
+    })
 }
 
 pub(crate) type NodeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a NodeTable<'a>>;

src/population_table.rs

@@ -20,20 +20,11 @@ impl PartialEq for PopulationTableRow {
 }
 
 fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option<PopulationTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            let table_ref = table.table_;
-            let rv = PopulationTableRow {
-                id: pos.into(),
-                metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
-            };
-            Some(rv)
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    let table_ref = table.table_;
+    Some(PopulationTableRow {
+        id: pos.into(),
+        metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
+    })
 }
 
 pub(crate) type PopulationTableRefIterator<'a> =

src/provenance.rs

@@ -45,19 +45,11 @@ impl std::fmt::Display for ProvenanceTableRow {
 }
 
 fn make_provenance_row(table: &ProvenanceTable, pos: tsk_id_t) -> Option<ProvenanceTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            Some(ProvenanceTableRow {
-                id: pos.into(),
-                timestamp: table.timestamp(pos).unwrap(),
-                record: table.record(pos).unwrap(),
-            })
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    Some(ProvenanceTableRow {
+        id: pos.into(),
+        timestamp: table.timestamp(pos).ok()?,
+        record: table.record(pos).ok()?,
+    })
 }
 
 type ProvenanceTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a ProvenanceTable<'a>>;

src/site_table.rs

@@ -25,22 +25,13 @@ impl PartialEq for SiteTableRow {
 }
 
 fn make_site_table_row(table: &SiteTable, pos: tsk_id_t) -> Option<SiteTableRow> {
-    if let Ok(p) = crate::SizeType::try_from(pos) {
-        if p < table.num_rows() {
-            let table_ref = table.table_;
-            let rv = SiteTableRow {
-                id: pos.into(),
-                position: table.position(pos).unwrap(),
-                ancestral_state: table.ancestral_state(pos).unwrap().map(|s| s.to_vec()),
-                metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
-            };
-            Some(rv)
-        } else {
-            None
-        }
-    } else {
-        None
-    }
+    let table_ref = table.table_;
+    Some(SiteTableRow {
+        id: pos.into(),
+        position: table.position(pos).ok()?,
+        ancestral_state: table.ancestral_state(pos).ok()?.map(|s| s.to_vec()),
+        metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
+    })
 }
 
 pub(crate) type SiteTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a SiteTable<'a>>;

tests/empty_table_collection.rs

@@ -0,0 +1,27 @@
+#[test]
+fn test_empty_table_collection() {
+    use tskit::TableAccess;
+
+    let tables = tskit::TableCollection::new(10.).unwrap();
+
+    assert!(tables.edges().row(-1).is_err());
+    assert!(tables.edges().row(0).is_err());
+    assert!(tables.nodes().row(-1).is_err());
+    assert!(tables.nodes().row(0).is_err());
+    assert!(tables.sites().row(-1).is_err());
+    assert!(tables.sites().row(0).is_err());
+    assert!(tables.mutations().row(-1).is_err());
+    assert!(tables.mutations().row(0).is_err());
+    assert!(tables.individuals().row(-1).is_err());
+    assert!(tables.individuals().row(0).is_err());
+    assert!(tables.populations().row(-1).is_err());
+    assert!(tables.populations().row(0).is_err());
+    assert!(tables.migrations().row(-1).is_err());
+    assert!(tables.migrations().row(0).is_err());
+
+    #[cfg(feature = "provenance")]
+    {
+        assert!(tables.provenances().row(-1).is_err());
+        assert!(tables.provenances().row(0).is_err());
+    }
+}