Skip to content

Commit 0787f9f

Browse files
molpopgenmolpopgen
authored
refactor: make column access fn in sys private (#713)
1 parent 972af66 commit 0787f9f

16 files changed

+170
-254
lines changed

‎examples/haploid_wright_fisher.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,11 @@ fn stress_test_total_branch_length() {
163163
if let Some(tree) = tree_iter.next() {
164164
let b = tree.total_branch_length(false).unwrap();
165165
let b2 = unsafe {
166-
tskit::bindings::tsk_tree_get_total_branch_length(tree.as_ll_ref(), -1, &mut x)
166+
tskit::bindings::tsk_tree_get_total_branch_length(
167+
tree.as_ll_ref(),
168+
-1,
169+
&mut x,
170+
)
167171
};
168172
assert!(b2 >= 0, "{}", b2);
169173
assert!(f64::from(b) - x <= 1e-8);

‎src/edge_table.rs‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,7 @@ impl EdgeTable {
223223
/// * `Some(parent)` if `u` is valid.
224224
/// * `None` otherwise.
225225
pub fn parent<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<NodeId> {
226-
assert!(self.num_rows() == 0 || !self.as_ref().parent.is_null());
227-
// SAFETY: either the column is empty or the point is not NULL
228-
unsafe {
229-
sys::tsk_column_access::<NodeId, _, _, _>(
230-
row.into(),
231-
self.as_ref().parent,
232-
self.num_rows(),
233-
)
234-
}
226+
self.table_.parent(row.into())
235227
}
236228

237229
/// Return the ``child`` value from row ``row`` of the table.
@@ -241,15 +233,7 @@ impl EdgeTable {
241233
/// * `Some(child)` if `u` is valid.
242234
/// * `None` otherwise.
243235
pub fn child<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<NodeId> {
244-
assert!(self.num_rows() == 0 || !self.as_ref().child.is_null());
245-
// SAFETY: either the column is empty or the point is not NULL
246-
unsafe {
247-
sys::tsk_column_access::<NodeId, _, _, _>(
248-
row.into(),
249-
self.as_ref().child,
250-
self.num_rows(),
251-
)
252-
}
236+
self.table_.child(row.into())
253237
}
254238

255239
/// Return the ``left`` value from row ``row`` of the table.
@@ -259,15 +243,7 @@ impl EdgeTable {
259243
/// * `Some(position)` if `u` is valid.
260244
/// * `None` otherwise.
261245
pub fn left<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<Position> {
262-
assert!(self.num_rows() == 0 || !self.as_ref().left.is_null());
263-
// SAFETY: either the column is empty or the point is not NULL
264-
unsafe {
265-
sys::tsk_column_access::<Position, _, _, _>(
266-
row.into(),
267-
self.as_ref().left,
268-
self.num_rows(),
269-
)
270-
}
246+
self.table_.left(row.into())
271247
}
272248

273249
/// Return the ``right`` value from row ``row`` of the table.
@@ -277,15 +253,7 @@ impl EdgeTable {
277253
/// * `Some(position)` if `u` is valid.
278254
/// * `None` otherwise.
279255
pub fn right<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<Position> {
280-
assert!(self.num_rows() == 0 || !self.as_ref().right.is_null());
281-
// SAFETY: either the column is empty or the point is not NULL
282-
unsafe {
283-
sys::tsk_column_access::<Position, _, _, _>(
284-
row.into(),
285-
self.as_ref().right,
286-
self.num_rows(),
287-
)
288-
}
256+
self.table_.right(row.into())
289257
}
290258

291259
/// Retrieve decoded metadata for a `row`.

‎src/individual_table.rs‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -209,15 +209,7 @@ impl IndividualTable {
209209
/// * `Some(flags)` if `row` is valid.
210210
/// * `None` otherwise.
211211
pub fn flags<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<IndividualFlags> {
212-
assert!(self.num_rows() == 0 || !self.as_ref().flags.is_null());
213-
// SAFETY: either the column is empty or the point is not NULL
214-
unsafe {
215-
sys::tsk_column_access::<IndividualFlags, _, _, _>(
216-
row.into(),
217-
self.as_ref().flags,
218-
self.num_rows(),
219-
)
220-
}
212+
self.table_.flags(row.into())
221213
}
222214

223215
/// Return the locations for a given row.

‎src/migration_table.rs‎

Lines changed: 7 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -237,16 +237,7 @@ impl MigrationTable {
237237
/// * `Some(position)` if `row` is valid.
238238
/// * `None` otherwise.
239239
pub fn left<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<Position> {
240-
assert!(self.num_rows() == 0 || !self.as_ref().time.is_null());
241-
// SAFETY: either the column is empty or the pointer is not null,
242-
// in which case the correct lengths are from the low-level objects
243-
unsafe {
244-
sys::tsk_column_access::<Position, _, _, _>(
245-
row.into(),
246-
self.as_ref().left,
247-
self.num_rows(),
248-
)
249-
}
240+
self.table_.left(row.into())
250241
}
251242

252243
/// Return the right coordinate for a given row.
@@ -256,35 +247,17 @@ impl MigrationTable {
256247
/// * `Some(positions)` if `row` is valid.
257248
/// * `None` otherwise.
258249
pub fn right<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<Position> {
259-
assert!(self.num_rows() == 0 || !self.as_ref().right.is_null());
260-
// SAFETY: either the column is empty or the pointer is not null,
261-
// in which case the correct lengths are from the low-level objects
262-
unsafe {
263-
sys::tsk_column_access::<Position, _, _, _>(
264-
row.into(),
265-
self.as_ref().right,
266-
self.num_rows(),
267-
)
268-
}
250+
self.table_.right(row.into())
269251
}
270252

271253
/// Return the node for a given row.
272254
///
273255
/// # Returns
274-
///
256+
//
275257
/// * `Some(node)` if `row` is valid.
276258
/// * `None` otherwise.
277259
pub fn node<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<NodeId> {
278-
assert!(self.num_rows() == 0 || !self.as_ref().node.is_null());
279-
// SAFETY: either the column is empty or the pointer is not null,
280-
// in which case the correct lengths are from the low-level objects
281-
unsafe {
282-
sys::tsk_column_access::<NodeId, _, _, _>(
283-
row.into(),
284-
self.as_ref().node,
285-
self.num_rows(),
286-
)
287-
}
260+
self.table_.node(row.into())
288261
}
289262

290263
/// Return the source population for a given row.
@@ -294,16 +267,7 @@ impl MigrationTable {
294267
/// * `Some(population)` if `row` is valid.
295268
/// * `None` otherwise.
296269
pub fn source<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<PopulationId> {
297-
assert!(self.num_rows() == 0 || !self.as_ref().time.is_null());
298-
// SAFETY: either the column is empty or the pointer is not null,
299-
// in which case the correct lengths are from the low-level objects
300-
unsafe {
301-
sys::tsk_column_access::<PopulationId, _, _, _>(
302-
row.into(),
303-
self.as_ref().source,
304-
self.num_rows(),
305-
)
306-
}
270+
self.table_.source(row.into())
307271
}
308272

309273
/// Return the destination population for a given row.
@@ -313,16 +277,7 @@ impl MigrationTable {
313277
/// * `Some(population)` if `row` is valid.
314278
/// * `None` otherwise.
315279
pub fn dest<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<PopulationId> {
316-
assert!(self.num_rows() == 0 || !self.as_ref().dest.is_null());
317-
// SAFETY: either the column is empty or the pointer is not null,
318-
// in which case the correct lengths are from the low-level objects
319-
unsafe {
320-
sys::tsk_column_access::<PopulationId, _, _, _>(
321-
row.into(),
322-
self.as_ref().dest,
323-
self.num_rows(),
324-
)
325-
}
280+
self.table_.dest(row.into())
326281
}
327282

328283
/// Return the time of the migration event for a given row.
@@ -332,12 +287,7 @@ impl MigrationTable {
332287
/// * `Some(time)` if `row` is valid.
333288
/// * `None` otherwise.
334289
pub fn time<M: Into<MigrationId> + Copy>(&self, row: M) -> Option<Time> {
335-
assert!(self.num_rows() == 0 || !self.as_ref().time.is_null());
336-
// SAFETY: either the column is empty or the pointer is not null,
337-
// in which case the correct lengths are from the low-level objects
338-
unsafe {
339-
sys::tsk_column_access::<Time, _, _, _>(row.into(), self.as_ref().time, self.num_rows())
340-
}
290+
self.table_.time(row.into())
341291
}
342292

343293
/// Retrieve decoded metadata for a `row`.

‎src/mutation_table.rs‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -233,16 +233,7 @@ impl MutationTable {
233233
/// Will return [``IndexError``](crate::TskitError::IndexError)
234234
/// if ``row`` is out of range.
235235
pub fn site<M: Into<MutationId> + Copy>(&self, row: M) -> Option<SiteId> {
236-
assert!(self.num_rows() == 0 || !self.as_ref().site.is_null());
237-
// SAFETY: either the column is empty or the pointer is not null,
238-
// in which case the correct lengths are from the low-level objects
239-
unsafe {
240-
sys::tsk_column_access::<SiteId, _, _, _>(
241-
row.into(),
242-
self.as_ref().site,
243-
self.num_rows(),
244-
)
245-
}
236+
self.table_.site(row.into())
246237
}
247238

248239
/// Return the ``node`` value from row ``row`` of the table.
@@ -252,16 +243,7 @@ impl MutationTable {
252243
/// Will return [``IndexError``](crate::TskitError::IndexError)
253244
/// if ``row`` is out of range.
254245
pub fn node<M: Into<MutationId> + Copy>(&self, row: M) -> Option<NodeId> {
255-
assert!(self.num_rows() == 0 || !self.as_ref().node.is_null());
256-
// SAFETY: either the column is empty or the pointer is not null,
257-
// in which case the correct lengths are from the low-level objects
258-
unsafe {
259-
sys::tsk_column_access::<NodeId, _, _, _>(
260-
row.into(),
261-
self.as_ref().node,
262-
self.num_rows(),
263-
)
264-
}
246+
self.table_.node(row.into())
265247
}
266248

267249
/// Return the ``parent`` value from row ``row`` of the table.
@@ -271,16 +253,7 @@ impl MutationTable {
271253
/// Will return [``IndexError``](crate::TskitError::IndexError)
272254
/// if ``row`` is out of range.
273255
pub fn parent<M: Into<MutationId> + Copy>(&self, row: M) -> Option<MutationId> {
274-
assert!(self.num_rows() == 0 || !self.as_ref().parent.is_null());
275-
// SAFETY: either the column is empty or the pointer is not null,
276-
// in which case the correct lengths are from the low-level objects
277-
unsafe {
278-
sys::tsk_column_access::<MutationId, _, _, _>(
279-
row.into(),
280-
self.as_ref().parent,
281-
self.num_rows(),
282-
)
283-
}
256+
self.table_.parent(row.into())
284257
}
285258

286259
/// Return the ``time`` value from row ``row`` of the table.
@@ -290,12 +263,7 @@ impl MutationTable {
290263
/// Will return [``IndexError``](crate::TskitError::IndexError)
291264
/// if ``row`` is out of range.
292265
pub fn time<M: Into<MutationId> + Copy>(&self, row: M) -> Option<Time> {
293-
assert!(self.num_rows() == 0 || !self.as_ref().time.is_null());
294-
// SAFETY: either the column is empty or the pointer is not null,
295-
// in which case the correct lengths are from the low-level objects
296-
unsafe {
297-
sys::tsk_column_access::<Time, _, _, _>(row.into(), self.as_ref().time, self.num_rows())
298-
}
266+
self.table_.time(row.into())
299267
}
300268

301269
/// Get the ``derived_state`` value from row ``row`` of the table.

‎src/node_table.rs‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -480,12 +480,7 @@ impl NodeTable {
480480
/// # }
481481
/// ```
482482
pub fn time<N: Into<NodeId> + Copy>(&self, row: N) -> Option<Time> {
483-
assert!(self.num_rows() == 0 || !self.as_ref().time.is_null());
484-
// SAFETY: either the column is empty or the pointer is not null,
485-
// in which case the correct lengths are from the low-level objects
486-
unsafe {
487-
sys::tsk_column_access::<Time, _, _, _>(row.into(), self.as_ref().time, self.num_rows())
488-
}
483+
self.table_.time(row.into())
489484
}
490485

491486
/// Return the ``flags`` value from row ``row`` of the table.
@@ -510,16 +505,7 @@ impl NodeTable {
510505
/// # }
511506
/// ```
512507
pub fn flags<N: Into<NodeId> + Copy>(&self, row: N) -> Option<NodeFlags> {
513-
assert!(self.num_rows() == 0 || !self.as_ref().flags.is_null());
514-
// SAFETY: either the column is empty or the pointer is not null,
515-
// in which case the correct lengths are from the low-level objects
516-
unsafe {
517-
sys::tsk_column_access::<NodeFlags, _, _, _>(
518-
row.into(),
519-
self.as_ref().flags,
520-
self.num_rows(),
521-
)
522-
}
508+
self.table_.flags(row.into())
523509
}
524510

525511
/// Return the ``population`` value from row ``row`` of the table.
@@ -544,16 +530,7 @@ impl NodeTable {
544530
/// * `Some(population)` if `row` is valid.
545531
/// * `None` otherwise.
546532
pub fn population<N: Into<NodeId> + Copy>(&self, row: N) -> Option<PopulationId> {
547-
assert!(self.num_rows() == 0 || !self.as_ref().population.is_null());
548-
// SAFETY: either the column is empty or the pointer is not null,
549-
// in which case the correct lengths are from the low-level objects
550-
unsafe {
551-
sys::tsk_column_access::<PopulationId, _, _, _>(
552-
row.into(),
553-
self.as_ref().population,
554-
self.num_rows(),
555-
)
556-
}
533+
self.table_.population(row.into())
557534
}
558535

559536
/// Return the ``population`` value from row ``row`` of the table.
@@ -592,16 +569,7 @@ impl NodeTable {
592569
/// * `Some(individual)` if `row` is valid.
593570
/// * `None` otherwise.
594571
pub fn individual<N: Into<NodeId> + Copy>(&self, row: N) -> Option<IndividualId> {
595-
assert!(self.num_rows() == 0 || !self.as_ref().individual.is_null());
596-
// SAFETY: either the column is empty or the pointer is not null,
597-
// in which case the correct lengths are from the low-level objects
598-
unsafe {
599-
sys::tsk_column_access::<IndividualId, _, _, _>(
600-
row.into(),
601-
self.as_ref().individual,
602-
self.num_rows(),
603-
)
604-
}
572+
self.table_.individual(row.into())
605573
}
606574

607575
/// Retrieve decoded metadata for a `row`.

‎src/site_table.rs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -202,14 +202,7 @@ impl SiteTable {
202202
/// * `Some(position)` if `row` is valid.
203203
/// * `None` otherwise.
204204
pub fn position<S: Into<SiteId> + Copy>(&self, row: S) -> Option<Position> {
205-
assert!(self.num_rows() == 0 || !self.as_ref().position.is_null());
206-
unsafe {
207-
sys::tsk_column_access::<Position, _, _, _>(
208-
row.into(),
209-
self.as_ref().position,
210-
self.num_rows(),
211-
)
212-
}
205+
self.table_.position(row.into())
213206
}
214207

215208
/// Get the ``ancestral_state`` value from row ``row`` of the table.

‎src/sys/edge_table.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
use std::ptr::NonNull;
22

3+
use super::newtypes::EdgeId;
4+
use super::newtypes::NodeId;
5+
use super::newtypes::Position;
6+
37
use super::bindings::tsk_edge_table_add_row;
48
use super::bindings::tsk_edge_table_clear;
59
use super::bindings::tsk_edge_table_init;
@@ -65,6 +69,22 @@ impl EdgeTable {
6569
))
6670
}
6771
}
72+
73+
pub fn parent(&self, row: EdgeId) -> Option<NodeId> {
74+
safe_tsk_column_access!(self, row, NodeId, parent)
75+
}
76+
77+
pub fn child(&self, row: EdgeId) -> Option<NodeId> {
78+
safe_tsk_column_access!(self, row, NodeId, child)
79+
}
80+
81+
pub fn left(&self, row: EdgeId) -> Option<Position> {
82+
safe_tsk_column_access!(self, row, Position, left)
83+
}
84+
85+
pub fn right(&self, row: EdgeId) -> Option<Position> {
86+
safe_tsk_column_access!(self, row, Position, right)
87+
}
6888
}
6989

7090
impl Default for EdgeTable {

0 commit comments

Comments
 (0)