diff --git a/src/_macros.rs b/src/_macros.rs index cd2e3c36a..2dc377ce4 100644 --- a/src/_macros.rs +++ b/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 {}) }; } diff --git a/src/edge_table.rs b/src/edge_table.rs index 6d9f38fdd..699ad72f5 100644 --- a/src/edge_table.rs +++ b/src/edge_table.rs @@ -27,24 +27,15 @@ impl PartialEq for EdgeTableRow { } fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option { - 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>>; diff --git a/src/migration_table.rs b/src/migration_table.rs index 7fbf54ddd..0e62da604 100644 --- a/src/migration_table.rs +++ b/src/migration_table.rs @@ -33,25 +33,17 @@ impl PartialEq for MigrationTableRow { } fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option { - 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> = diff --git a/src/mutation_table.rs b/src/mutation_table.rs index be4bdf6e5..8c33b8475 100644 --- a/src/mutation_table.rs +++ b/src/mutation_table.rs @@ -30,25 +30,16 @@ impl PartialEq for MutationTableRow { } fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option { - 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>>; diff --git a/src/node_table.rs b/src/node_table.rs index 52b2b4f6a..d6c281ff5 100644 --- a/src/node_table.rs +++ b/src/node_table.rs @@ -29,23 +29,15 @@ impl PartialEq for NodeTableRow { } fn make_node_table_row(table: &NodeTable, pos: tsk_id_t) -> Option { - 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>>; diff --git a/src/population_table.rs b/src/population_table.rs index 42c11fc6e..c81787c62 100644 --- a/src/population_table.rs +++ b/src/population_table.rs @@ -20,20 +20,11 @@ impl PartialEq for PopulationTableRow { } fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option { - 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> = diff --git a/src/provenance.rs b/src/provenance.rs index 7a84a491e..c132fa571 100644 --- a/src/provenance.rs +++ b/src/provenance.rs @@ -45,19 +45,11 @@ impl std::fmt::Display for ProvenanceTableRow { } fn make_provenance_row(table: &ProvenanceTable, pos: tsk_id_t) -> Option { - 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>>; diff --git a/src/site_table.rs b/src/site_table.rs index add82e5ad..826ad5e00 100644 --- a/src/site_table.rs +++ b/src/site_table.rs @@ -25,22 +25,13 @@ impl PartialEq for SiteTableRow { } fn make_site_table_row(table: &SiteTable, pos: tsk_id_t) -> Option { - 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>>; diff --git a/tests/empty_table_collection.rs b/tests/empty_table_collection.rs new file mode 100644 index 000000000..06aafd204 --- /dev/null +++ b/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()); + } +}