Skip to content

Commit dccfdf9

Browse files
molpopgenmolpopgen
authored
refactor: streamline code for generating table rows. (#340)
* liberal use of Result<_>::ok()? and Option<_>::ok_or_else(...)
1 parent 6c0764c commit dccfdf9

File tree

9 files changed

+86
-128
lines changed

9 files changed

+86
-128
lines changed

‎src/_macros.rs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,7 @@ macro_rules! decode_metadata_row {
153153

154154
macro_rules! table_row_decode_metadata {
155155
($owner: ident, $table: ident, $pos: ident) => {
156-
metadata_to_vector!($owner, $table, $pos)
157-
.unwrap()
158-
.map(|x| x)
156+
metadata_to_vector!($owner, $table, $pos).ok()?.map(|x| x)
159157
};
160158
}
161159

@@ -184,17 +182,10 @@ macro_rules! err_if_not_tracking_samples {
184182
// This macro assumes that table row access helper
185183
// functions have a standard interface.
186184
// Here, we convert the None type to an Error,
187-
// as it applies $row is out of range.
185+
// as it implies $row is out of range.
188186
macro_rules! table_row_access {
189187
($row: expr, $table: expr, $row_fn: ident) => {
190-
if $row < 0 {
191-
Err(TskitError::IndexError)
192-
} else {
193-
match $row_fn($table, $row) {
194-
Some(x) => Ok(x),
195-
None => Err(TskitError::IndexError),
196-
}
197-
}
188+
$row_fn($table, $row).ok_or_else(|| TskitError::IndexError {})
198189
};
199190
}
200191

‎src/edge_table.rs‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,15 @@ impl PartialEq for EdgeTableRow {
2727
}
2828

2929
fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow> {
30-
if let Ok(p) = crate::SizeType::try_from(pos) {
31-
if p < table.num_rows() {
32-
let table_ref = table.table_;
33-
let rv = EdgeTableRow {
34-
id: pos.into(),
35-
left: table.left(pos).unwrap(),
36-
right: table.right(pos).unwrap(),
37-
parent: table.parent(pos).unwrap(),
38-
child: table.child(pos).unwrap(),
39-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
40-
};
41-
Some(rv)
42-
} else {
43-
None
44-
}
45-
} else {
46-
None
47-
}
30+
let table_ref = table.table_;
31+
Some(EdgeTableRow {
32+
id: pos.into(),
33+
left: table.left(pos).ok()?,
34+
right: table.right(pos).ok()?,
35+
parent: table.parent(pos).ok()?,
36+
child: table.child(pos).ok()?,
37+
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
38+
})
4839
}
4940

5041
pub(crate) type EdgeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a EdgeTable<'a>>;

‎src/migration_table.rs‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,17 @@ impl PartialEq for MigrationTableRow {
3333
}
3434

3535
fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<MigrationTableRow> {
36-
if let Ok(p) = crate::SizeType::try_from(pos) {
37-
if p < table.num_rows() {
38-
let table_ref = table.table_;
39-
Some(MigrationTableRow {
40-
id: pos.into(),
41-
left: table.left(pos).unwrap(),
42-
right: table.right(pos).unwrap(),
43-
node: table.node(pos).unwrap(),
44-
source: table.source(pos).unwrap(),
45-
dest: table.dest(pos).unwrap(),
46-
time: table.time(pos).unwrap(),
47-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
48-
})
49-
} else {
50-
None
51-
}
52-
} else {
53-
None
54-
}
36+
let table_ref = table.table_;
37+
Some(MigrationTableRow {
38+
id: pos.into(),
39+
left: table.left(pos).ok()?,
40+
right: table.right(pos).ok()?,
41+
node: table.node(pos).ok()?,
42+
source: table.source(pos).ok()?,
43+
dest: table.dest(pos).ok()?,
44+
time: table.time(pos).ok()?,
45+
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
46+
})
5547
}
5648

5749
pub(crate) type MigrationTableRefIterator<'a> =

‎src/mutation_table.rs‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,16 @@ impl PartialEq for MutationTableRow {
3030
}
3131

3232
fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option<MutationTableRow> {
33-
if let Ok(p) = crate::SizeType::try_from(pos) {
34-
if p < table.num_rows() {
35-
let table_ref = table.table_;
36-
let rv = MutationTableRow {
37-
id: pos.into(),
38-
site: table.site(pos).unwrap(),
39-
node: table.node(pos).unwrap(),
40-
parent: table.parent(pos).unwrap(),
41-
time: table.time(pos).unwrap(),
42-
derived_state: table.derived_state(pos).unwrap().map(|s| s.to_vec()),
43-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
44-
};
45-
Some(rv)
46-
} else {
47-
None
48-
}
49-
} else {
50-
None
51-
}
33+
let table_ref = table.table_;
34+
Some(MutationTableRow {
35+
id: pos.into(),
36+
site: table.site(pos).ok()?,
37+
node: table.node(pos).ok()?,
38+
parent: table.parent(pos).ok()?,
39+
time: table.time(pos).ok()?,
40+
derived_state: table.derived_state(pos).ok()?.map(|s| s.to_vec()),
41+
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
42+
})
5243
}
5344
pub(crate) type MutationTableRefIterator<'a> =
5445
crate::table_iterator::TableIterator<&'a MutationTable<'a>>;

‎src/node_table.rs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,15 @@ impl PartialEq for NodeTableRow {
2929
}
3030

3131
fn make_node_table_row(table: &NodeTable, pos: tsk_id_t) -> Option<NodeTableRow> {
32-
if let Ok(p) = crate::SizeType::try_from(pos) {
33-
if p < table.num_rows() {
34-
let table_ref = table.table_;
35-
Some(NodeTableRow {
36-
id: pos.into(),
37-
time: table.time(pos).unwrap(),
38-
flags: table.flags(pos).unwrap(),
39-
population: table.population(pos).unwrap(),
40-
individual: table.individual(pos).unwrap(),
41-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
42-
})
43-
} else {
44-
None
45-
}
46-
} else {
47-
None
48-
}
32+
let table_ref = table.table_;
33+
Some(NodeTableRow {
34+
id: pos.into(),
35+
time: table.time(pos).ok()?,
36+
flags: table.flags(pos).ok()?,
37+
population: table.population(pos).ok()?,
38+
individual: table.individual(pos).ok()?,
39+
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
40+
})
4941
}
5042

5143
pub(crate) type NodeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a NodeTable<'a>>;

‎src/population_table.rs‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,11 @@ impl PartialEq for PopulationTableRow {
2020
}
2121

2222
fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option<PopulationTableRow> {
23-
if let Ok(p) = crate::SizeType::try_from(pos) {
24-
if p < table.num_rows() {
25-
let table_ref = table.table_;
26-
let rv = PopulationTableRow {
27-
id: pos.into(),
28-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
29-
};
30-
Some(rv)
31-
} else {
32-
None
33-
}
34-
} else {
35-
None
36-
}
23+
let table_ref = table.table_;
24+
Some(PopulationTableRow {
25+
id: pos.into(),
26+
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
27+
})
3728
}
3829

3930
pub(crate) type PopulationTableRefIterator<'a> =

‎src/provenance.rs‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,11 @@ impl std::fmt::Display for ProvenanceTableRow {
4545
}
4646

4747
fn make_provenance_row(table: &ProvenanceTable, pos: tsk_id_t) -> Option<ProvenanceTableRow> {
48-
if let Ok(p) = crate::SizeType::try_from(pos) {
49-
if p < table.num_rows() {
50-
Some(ProvenanceTableRow {
51-
id: pos.into(),
52-
timestamp: table.timestamp(pos).unwrap(),
53-
record: table.record(pos).unwrap(),
54-
})
55-
} else {
56-
None
57-
}
58-
} else {
59-
None
60-
}
48+
Some(ProvenanceTableRow {
49+
id: pos.into(),
50+
timestamp: table.timestamp(pos).ok()?,
51+
record: table.record(pos).ok()?,
52+
})
6153
}
6254

6355
type ProvenanceTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a ProvenanceTable<'a>>;

‎src/site_table.rs‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,13 @@ impl PartialEq for SiteTableRow {
2525
}
2626

2727
fn make_site_table_row(table: &SiteTable, pos: tsk_id_t) -> Option<SiteTableRow> {
28-
if let Ok(p) = crate::SizeType::try_from(pos) {
29-
if p < table.num_rows() {
30-
let table_ref = table.table_;
31-
let rv = SiteTableRow {
32-
id: pos.into(),
33-
position: table.position(pos).unwrap(),
34-
ancestral_state: table.ancestral_state(pos).unwrap().map(|s| s.to_vec()),
35-
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
36-
};
37-
Some(rv)
38-
} else {
39-
None
40-
}
41-
} else {
42-
None
43-
}
28+
let table_ref = table.table_;
29+
Some(SiteTableRow {
30+
id: pos.into(),
31+
position: table.position(pos).ok()?,
32+
ancestral_state: table.ancestral_state(pos).ok()?.map(|s| s.to_vec()),
33+
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
34+
})
4435
}
4536

4637
pub(crate) type SiteTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a SiteTable<'a>>;

‎tests/empty_table_collection.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#[test]
2+
fn test_empty_table_collection() {
3+
use tskit::TableAccess;
4+
5+
let tables = tskit::TableCollection::new(10.).unwrap();
6+
7+
assert!(tables.edges().row(-1).is_err());
8+
assert!(tables.edges().row(0).is_err());
9+
assert!(tables.nodes().row(-1).is_err());
10+
assert!(tables.nodes().row(0).is_err());
11+
assert!(tables.sites().row(-1).is_err());
12+
assert!(tables.sites().row(0).is_err());
13+
assert!(tables.mutations().row(-1).is_err());
14+
assert!(tables.mutations().row(0).is_err());
15+
assert!(tables.individuals().row(-1).is_err());
16+
assert!(tables.individuals().row(0).is_err());
17+
assert!(tables.populations().row(-1).is_err());
18+
assert!(tables.populations().row(0).is_err());
19+
assert!(tables.migrations().row(-1).is_err());
20+
assert!(tables.migrations().row(0).is_err());
21+
22+
#[cfg(feature = "provenance")]
23+
{
24+
assert!(tables.provenances().row(-1).is_err());
25+
assert!(tables.provenances().row(0).is_err());
26+
}
27+
}

0 commit comments

Comments
 (0)