Skip to content

Commit 11e6c4c

Browse files
molpopgenmolpopgen
committed
refactor: return None for out-of-range row indexes, part II.
* table column getters now return Option instead of Result * tree getters now return Option instead of Result * Tree functions returning iterators now return Option<impl Iterator<...>> instead of Result<impl Iterator<...>>. * Update documentation BREAKING CHANGE: return value changes
1 parent 48d820a commit 11e6c4c

11 files changed

+257
-289
lines changed

‎examples/forward_simulation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ fn update_bookmark(
373373
for a in alive {
374374
for node in [a.node0, a.node1] {
375375
match nodes.time(node) {
376-
Ok(time) => {
376+
Some(time) => {
377377
most_recent_birth_time = if time < most_recent_birth_time {
378378
time.into()
379379
} else {
@@ -385,7 +385,7 @@ fn update_bookmark(
385385
most_ancient_birth_time
386386
};
387387
}
388-
Err(e) => return Err(e),
388+
None => return Err(tskit::TskitError::IndexError {}),
389389
}
390390
}
391391
}

‎src/_macros.rs

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,33 @@ macro_rules! panic_on_tskit_error {
3030
macro_rules! unsafe_tsk_column_access {
3131
($i: expr, $lo: expr, $hi: expr, $array: expr) => {{
3232
if $i < $lo || ($i as $crate::tsk_size_t) >= $hi {
33-
Err($crate::error::TskitError::IndexError {})
33+
None
3434
} else {
35-
Ok(unsafe { *$array.offset($i as isize) })
35+
Some(unsafe { *$array.offset($i as isize) })
3636
}
3737
}};
3838
($i: expr, $lo: expr, $hi: expr, $array: expr, $output_id_type: expr) => {{
3939
if $i < $lo || ($i as $crate::tsk_size_t) >= $hi {
40-
Err($crate::error::TskitError::IndexError {})
40+
None
4141
} else {
42-
Ok($output_id_type(unsafe { *$array.offset($i as isize) }))
42+
Some($output_id_type(unsafe { *$array.offset($i as isize) }))
4343
}
4444
}};
4545
}
4646

47+
macro_rules! unsafe_tsk_column_access_and_map_into {
48+
($i: expr, $lo: expr, $hi: expr, $array: expr) => {{
49+
unsafe_tsk_column_access!($i, $lo, $hi, $array).map(|v| v.into())
50+
}};
51+
}
52+
4753
macro_rules! unsafe_tsk_ragged_column_access {
4854
($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr) => {{
49-
let i = $crate::SizeType::try_from($i)?;
55+
let i = $crate::SizeType::try_from($i).ok()?;
5056
if $i < $lo || i >= $hi {
51-
Err(TskitError::IndexError {})
57+
None
5258
} else if $offset_array_len == 0 {
53-
Ok(None)
59+
None
5460
} else {
5561
let start = unsafe { *$offset_array.offset($i as isize) };
5662
let stop = if i < $hi {
@@ -59,23 +65,23 @@ macro_rules! unsafe_tsk_ragged_column_access {
5965
$offset_array_len as tsk_size_t
6066
};
6167
if start == stop {
62-
Ok(None)
68+
None
6369
} else {
6470
let mut buffer = vec![];
6571
for i in start..stop {
6672
buffer.push(unsafe { *$array.offset(i as isize) });
6773
}
68-
Ok(Some(buffer))
74+
Some(buffer)
6975
}
7076
}
7177
}};
7278

7379
($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr, $output_id_type: ty) => {{
74-
let i = $crate::SizeType::try_from($i)?;
80+
let i = $crate::SizeType::try_from($i).ok()?;
7581
if $i < $lo || i >= $hi {
76-
Err(TskitError::IndexError {})
82+
None
7783
} else if $offset_array_len == 0 {
78-
Ok(None)
84+
None
7985
} else {
8086
let start = unsafe { *$offset_array.offset($i as isize) };
8187
let stop = if i < $hi {
@@ -84,14 +90,14 @@ macro_rules! unsafe_tsk_ragged_column_access {
8490
$offset_array_len as tsk_size_t
8591
};
8692
if start == stop {
87-
Ok(None)
93+
None
8894
} else {
89-
Ok(Some(unsafe {
95+
Some(unsafe {
9096
std::slice::from_raw_parts(
9197
$array.offset(start as isize) as *const $output_id_type,
9298
stop as usize - start as usize,
9399
)
94-
}))
100+
})
95101
}
96102
}
97103
}};

‎src/edge_table.rs

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow>
3030
let table_ref = table.table_;
3131
Some(EdgeTableRow {
3232
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()?,
33+
left: table.left(pos)?,
34+
right: table.right(pos)?,
35+
parent: table.parent(pos)?,
36+
child: table.child(pos)?,
3737
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
3838
})
3939
}
@@ -83,48 +83,42 @@ impl<'a> EdgeTable<'a> {
8383

8484
/// Return the ``parent`` value from row ``row`` of the table.
8585
///
86-
/// # Errors
86+
/// # Returns
8787
///
88-
/// Will return [``IndexError``](crate::TskitError::IndexError)
89-
/// if ``row`` is out of range.
90-
pub fn parent<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<NodeId, TskitError> {
88+
/// * `Some(parent)` if `u` is valid.
89+
/// * `None` otherwise.
90+
pub fn parent<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<NodeId> {
9191
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.parent, NodeId)
9292
}
9393

9494
/// Return the ``child`` value from row ``row`` of the table.
9595
///
96-
/// # Errors
96+
/// # Returns
9797
///
98-
/// Will return [``IndexError``](crate::TskitError::IndexError)
99-
/// if ``row`` is out of range.
100-
pub fn child<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<NodeId, TskitError> {
98+
/// * `Some(child)` if `u` is valid.
99+
/// * `None` otherwise.
100+
pub fn child<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<NodeId> {
101101
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.child, NodeId)
102102
}
103103

104104
/// Return the ``left`` value from row ``row`` of the table.
105105
///
106-
/// # Errors
106+
/// # Returns
107107
///
108-
/// Will return [``IndexError``](crate::TskitError::IndexError)
109-
/// if ``row`` is out of range.
110-
pub fn left<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<Position, TskitError> {
111-
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left) {
112-
Ok(p) => Ok(p.into()),
113-
Err(e) => Err(e),
114-
}
108+
/// * `Some(position)` if `u` is valid.
109+
/// * `None` otherwise.
110+
pub fn left<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<Position> {
111+
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left, Position)
115112
}
116113

117114
/// Return the ``right`` value from row ``row`` of the table.
118115
///
119-
/// # Errors
116+
/// # Returns
120117
///
121-
/// Will return [``IndexError``](crate::TskitError::IndexError)
122-
/// if ``row`` is out of range.
123-
pub fn right<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Result<Position, TskitError> {
124-
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.right) {
125-
Ok(p) => Ok(p.into()),
126-
Err(e) => Err(e),
127-
}
118+
/// * `Some(position)` if `u` is valid.
119+
/// * `None` otherwise.
120+
pub fn right<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<Position> {
121+
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.right)
128122
}
129123

130124
pub fn metadata<T: metadata::MetadataRoundtrip>(

‎src/individual_table.rs

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<I
5555
let table_ref = table.table_;
5656
Some(IndividualTableRow {
5757
id: pos.into(),
58-
flags: table.flags(pos).ok()?,
59-
location: table.location(pos).ok()?.map(|s| s.to_vec()),
60-
parents: table.parents(pos).ok()?.map(|s| s.to_vec()),
58+
flags: table.flags(pos)?,
59+
location: table.location(pos).map(|s| s.to_vec()),
60+
parents: table.parents(pos).map(|s| s.to_vec()),
6161
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
6262
})
6363
}
@@ -101,28 +101,21 @@ impl<'a> IndividualTable<'a> {
101101

102102
/// Return the flags for a given row.
103103
///
104-
/// # Errors
104+
/// # Returns
105105
///
106-
/// * [`TskitError::IndexError`] if `row` is out of range.
107-
pub fn flags<I: Into<IndividualId> + Copy>(
108-
&self,
109-
row: I,
110-
) -> Result<IndividualFlags, TskitError> {
111-
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.flags) {
112-
Ok(f) => Ok(IndividualFlags::from(f)),
113-
Err(e) => Err(e),
114-
}
106+
/// * `Some(flags)` if `row` is valid.
107+
/// * `None` otherwise.
108+
pub fn flags<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<IndividualFlags> {
109+
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.flags)
115110
}
116111

117112
/// Return the locations for a given row.
118113
///
119-
/// # Errors
114+
/// # Returns
120115
///
121-
/// * [`TskitError::IndexError`] if `row` is out of range.
122-
pub fn location<I: Into<IndividualId> + Copy>(
123-
&self,
124-
row: I,
125-
) -> Result<Option<&[Location]>, TskitError> {
116+
/// * `Some(location)` if `row` is valid.
117+
/// * `None` otherwise.
118+
pub fn location<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<&[Location]> {
126119
unsafe_tsk_ragged_column_access!(
127120
row.into().0,
128121
0,
@@ -136,13 +129,11 @@ impl<'a> IndividualTable<'a> {
136129

137130
/// Return the parents for a given row.
138131
///
139-
/// # Errors
132+
/// # Returns
140133
///
141-
/// * [`TskitError::IndexError`] if `row` is out of range.
142-
pub fn parents<I: Into<IndividualId> + Copy>(
143-
&self,
144-
row: I,
145-
) -> Result<Option<&[IndividualId]>, TskitError> {
134+
/// * `Some(parents)` if `row` is valid.
135+
/// * `None` otherwise.
136+
pub fn parents<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<&[IndividualId]> {
146137
unsafe_tsk_ragged_column_access!(
147138
row.into().0,
148139
0,

‎src/migration_table.rs

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option<Mig
3636
let table_ref = table.table_;
3737
Some(MigrationTableRow {
3838
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()?,
39+
left: table.left(pos)?,
40+
right: table.right(pos)?,
41+
node: table.node(pos)?,
42+
source: table.source(pos)?,
43+
dest: table.dest(pos)?,
44+
time: table.time(pos)?,
4545
metadata: table_row_decode_metadata!(table, table_ref, pos).map(|m| m.to_vec()),
4646
})
4747
}
@@ -92,46 +92,41 @@ impl<'a> MigrationTable<'a> {
9292

9393
/// Return the left coordinate for a given row.
9494
///
95-
/// # Errors
95+
/// # Returns
9696
///
97-
/// * [`TskitError::IndexError`] if `row` is out of range.
98-
pub fn left<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<Position, TskitError> {
99-
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.left) {
100-
Ok(p) => Ok(p.into()),
101-
Err(e) => Err(e),
102-
}
97+
/// * `Some(position)` if `row` is valid.
98+
/// * `None` otherwise.
99+
pub fn left<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<Position> {
100+
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.left)
103101
}
104102

105103
/// Return the right coordinate for a given row.
106104
///
107-
/// # Errors
105+
/// # Returns
108106
///
109-
/// * [`TskitError::IndexError`] if `row` is out of range.
110-
pub fn right<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<Position, TskitError> {
111-
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.right) {
112-
Ok(p) => Ok(p.into()),
113-
Err(e) => Err(e),
114-
}
107+
/// * `Some(positions)` if `row` is valid.
108+
/// * `None` otherwise.
109+
pub fn right<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<Position> {
110+
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.right)
115111
}
116112

117113
/// Return the node for a given row.
118114
///
119-
/// # Errors
115+
/// # Returns
120116
///
121-
/// * [`TskitError::IndexError`] if `row` is out of range.
122-
pub fn node<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<NodeId, TskitError> {
117+
/// * `Some(node)` if `row` is valid.
118+
/// * `None` otherwise.
119+
pub fn node<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<NodeId> {
123120
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.node, NodeId)
124121
}
125122

126123
/// Return the source population for a given row.
127124
///
128-
/// # Errors
125+
/// # Returns
129126
///
130-
/// * [`TskitError::IndexError`] if `row` is out of range.
131-
pub fn source<M: Into<MigrationId> + Copy>(
132-
&'a self,
133-
row: M,
134-
) -> Result<PopulationId, TskitError> {
127+
/// * `Some(population)` if `row` is valid.
128+
/// * `None` otherwise.
129+
pub fn source<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<PopulationId> {
135130
unsafe_tsk_column_access!(
136131
row.into().0,
137132
0,
@@ -143,10 +138,11 @@ impl<'a> MigrationTable<'a> {
143138

144139
/// Return the destination population for a given row.
145140
///
146-
/// # Errors
141+
/// # Returns
147142
///
148-
/// * [`TskitError::IndexError`] if `row` is out of range.
149-
pub fn dest<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<PopulationId, TskitError> {
143+
/// * `Some(population)` if `row` is valid.
144+
/// * `None` otherwise.
145+
pub fn dest<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<PopulationId> {
150146
unsafe_tsk_column_access!(
151147
row.into().0,
152148
0,
@@ -158,21 +154,21 @@ impl<'a> MigrationTable<'a> {
158154

159155
/// Return the time of the migration event for a given row.
160156
///
161-
/// # Errors
157+
/// # Returns
162158
///
163-
/// * [`TskitError::IndexError`] if `row` is out of range.
164-
pub fn time<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Result<Time, TskitError> {
165-
match unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_.time) {
166-
Ok(t) => Ok(t.into()),
167-
Err(e) => Err(e),
168-
}
159+
/// * `Some(time)` if `row` is valid.
160+
/// * `None` otherwise.
161+
pub fn time<M: Into<MigrationId> + Copy>(&'a self, row: M) -> Option<Time> {
162+
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_.time)
169163
}
170164

171165
/// Return the metadata for a given row.
172166
///
173-
/// # Errors
167+
/// # Returns
174168
///
175-
/// * [`TskitError::IndexError`] if `row` is out of range.
169+
/// * `Some(Ok(metadata))` if `row` is valid and decoding succeeded
170+
/// * `Some(Err(_))` if `row` is valid and decoding failed.
171+
/// * `None` if `row` is not valid.
176172
pub fn metadata<T: metadata::MetadataRoundtrip>(
177173
&'a self,
178174
row: MigrationId,

0 commit comments

Comments
 (0)