Skip to content

Commit 58ac4a9

Browse files
molpopgenmolpopgen
authored
Refine what "NULL"-ness means for an Id type: (#154)
* All Id newtypes have a NULL const. * TSK_NULL is now pub(crate) instead of pub. * All internal use of TSK_NULL is removed except for initializing the NULL consts
1 parent ad4975f commit 58ac4a9

12 files changed

+138
-89
lines changed

‎examples/forward_simulation.rs

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -289,16 +289,21 @@ fn births(
289289
for p in parents {
290290
// Register the two nodes for our offspring
291291
let node0 = match tables.add_node(
292-
0, // flags
293-
birth_time as f64, // time
294-
tskit::TSK_NULL, // population
292+
0, // flags
293+
birth_time as f64, // time
294+
tskit::PopulationId::NULL, // population
295295
// individual
296-
tskit::TSK_NULL,
296+
tskit::IndividualId::NULL,
297297
) {
298298
Ok(x) => x,
299299
Err(e) => panic!("{}", e),
300300
};
301-
let node1 = match tables.add_node(0, birth_time as f64, tskit::TSK_NULL, tskit::TSK_NULL) {
301+
let node1 = match tables.add_node(
302+
0,
303+
birth_time as f64,
304+
tskit::PopulationId::NULL,
305+
tskit::IndividualId::NULL,
306+
) {
302307
Ok(x) => x,
303308
Err(e) => panic!("{}", e),
304309
};
@@ -333,9 +338,9 @@ fn simplify(alive: &mut [Diploid], tables: &mut tskit::TableCollection) {
333338
Some(idmap) => {
334339
for a in alive.iter_mut() {
335340
a.node0 = idmap[usize::from(a.node0)];
336-
assert!(a.node0 != tskit::TSK_NULL);
341+
assert!(a.node0 != tskit::NodeId::NULL);
337342
a.node1 = idmap[usize::from(a.node1)];
338-
assert!(a.node1 != tskit::TSK_NULL);
343+
assert!(a.node1 != tskit::NodeId::NULL);
339344
}
340345
}
341346
None => panic!("Unexpected None"),
@@ -354,13 +359,21 @@ fn runsim(params: &SimParams) -> tskit::TableCollection {
354359

355360
let mut alive: Vec<Diploid> = vec![];
356361
for _ in 0..params.popsize {
357-
let node0 = match tables.add_node(0, params.nsteps as f64, tskit::TSK_NULL, tskit::TSK_NULL)
358-
{
362+
let node0 = match tables.add_node(
363+
0,
364+
params.nsteps as f64,
365+
tskit::PopulationId::NULL,
366+
tskit::IndividualId::NULL,
367+
) {
359368
Ok(x) => x,
360369
Err(e) => panic!("{}", e),
361370
};
362-
let node1 = match tables.add_node(0, params.nsteps as f64, tskit::TSK_NULL, tskit::TSK_NULL)
363-
{
371+
let node1 = match tables.add_node(
372+
0,
373+
params.nsteps as f64,
374+
tskit::PopulationId::NULL,
375+
tskit::IndividualId::NULL,
376+
) {
364377
Ok(x) => x,
365378
Err(e) => panic!("{}", e),
366379
};

‎examples/tree_traversals.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use tskit::prelude::*;
55
fn traverse_upwards(tree: &tskit::Tree) {
66
for &s in tree.sample_nodes() {
77
let mut u = s;
8-
while u != tskit::TSK_NULL {
8+
while u != tskit::NodeId::NULL {
99
u = tree.parent(u).unwrap();
1010
}
1111
}

‎src/_macros.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,11 @@ macro_rules! tree_array_slice {
256256

257257
macro_rules! impl_id_traits {
258258
($idtype: ty) => {
259+
impl $idtype {
260+
/// NULL value for this type.
261+
pub const NULL: $idtype = Self($crate::TSK_NULL);
262+
}
263+
259264
impl From<$crate::tsk_id_t> for $idtype {
260265
fn from(value: $crate::tsk_id_t) -> Self {
261266
Self(value)
@@ -264,7 +269,7 @@ macro_rules! impl_id_traits {
264269

265270
impl $crate::IdIsNull for $idtype {
266271
fn is_null(&self) -> bool {
267-
self.0 == $crate::TSK_NULL
272+
self.0 == Self::NULL
268273
}
269274
}
270275

‎src/lib.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,7 @@ impl_id_traits!(EdgeId);
208208
// in a macro. bindgen thus misses it.
209209
// See bindgen issue 316.
210210
/// "Null" identifier value.
211-
pub const TSK_NULL: tsk_id_t = -1;
212-
213-
/// "Null" identifier value for [``NodeId``]
214-
pub const NULL_NODE_ID: NodeId = NodeId(-1);
211+
pub(crate) const TSK_NULL: tsk_id_t = -1;
215212

216213
pub use edge_table::{EdgeTable, EdgeTableRow};
217214
pub use error::TskitError;

‎src/metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ use thiserror::Error;
4545
/// dominance: 0.25};
4646
///
4747
/// // Add table row with metadata.
48-
/// tables.add_mutation_with_metadata(0, 0, tskit::TSK_NULL, 100., None,
48+
/// tables.add_mutation_with_metadata(0, 0, tskit::MutationId::NULL, 100., None,
4949
/// Some(&mutation)).unwrap();
5050
///
5151
/// // Decode the metadata

‎src/prelude.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ pub use crate::tsk_size_t;
66
pub use crate::NodeListGenerator;
77
pub use crate::TableAccess;
88
pub use crate::TskitTypeAccess;
9-
pub use crate::NULL_NODE_ID;
109
pub use crate::TSK_NODE_IS_SAMPLE;
11-
pub use crate::TSK_NULL;
1210
pub use streaming_iterator::DoubleEndedStreamingIterator;
1311
pub use streaming_iterator::StreamingIterator;

‎src/table_collection.rs

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::TableSortOptions;
1919
use crate::TreeSequenceFlags;
2020
use crate::TskReturnValue;
2121
use crate::TskitTypeAccess;
22-
use crate::{tsk_flags_t, tsk_id_t, tsk_size_t, TSK_NULL};
22+
use crate::{tsk_flags_t, tsk_id_t, tsk_size_t};
2323
use crate::{EdgeId, IndividualId, MigrationId, MutationId, NodeId, PopulationId, SiteId};
2424
use ll_bindings::tsk_table_collection_free;
2525

@@ -42,7 +42,7 @@ use ll_bindings::tsk_table_collection_free;
4242
///
4343
/// // Add node:
4444
///
45-
/// let rv = tables.add_node(0, 3.2, tskit::TSK_NULL, tskit::TSK_NULL).unwrap();
45+
/// let rv = tables.add_node(0, 3.2, tskit::PopulationId::NULL, tskit::IndividualId::NULL).unwrap();
4646
///
4747
/// // Get immutable reference to edge table
4848
/// let edges = tables.edges();
@@ -656,7 +656,7 @@ impl TableCollection {
656656
/// the behavior of simplification.
657657
/// * `idmap`: if `true`, the return value contains a vector equal
658658
/// in length to the input node table. For each input node,
659-
/// this vector either contains the node's new index or [`TSK_NULL`]
659+
/// this vector either contains the node's new index or [`NodeId::NULL`]
660660
/// if the input node is not part of the simplified history.
661661
pub fn simplify<N: Into<NodeId>>(
662662
&mut self,
@@ -666,7 +666,7 @@ impl TableCollection {
666666
) -> Result<Option<Vec<NodeId>>, TskitError> {
667667
let mut output_node_map: Vec<NodeId> = vec![];
668668
if idmap {
669-
output_node_map.resize(self.nodes().num_rows() as usize, TSK_NULL.into());
669+
output_node_map.resize(self.nodes().num_rows() as usize, NodeId::NULL);
670670
}
671671
let rv = unsafe {
672672
ll_bindings::tsk_table_collection_simplify(
@@ -746,13 +746,18 @@ impl crate::provenance::Provenance for TableCollection {
746746
#[cfg(test)]
747747
mod test {
748748
use super::*;
749-
use crate::TSK_NULL;
750749

751750
fn make_small_table_collection() -> TableCollection {
752751
let mut tables = TableCollection::new(1000.).unwrap();
753-
tables.add_node(0, 1.0, TSK_NULL, TSK_NULL).unwrap();
754-
tables.add_node(0, 0.0, TSK_NULL, TSK_NULL).unwrap();
755-
tables.add_node(0, 0.0, TSK_NULL, TSK_NULL).unwrap();
752+
tables
753+
.add_node(0, 1.0, PopulationId::NULL, IndividualId::NULL)
754+
.unwrap();
755+
tables
756+
.add_node(0, 0.0, PopulationId::NULL, IndividualId::NULL)
757+
.unwrap();
758+
tables
759+
.add_node(0, 0.0, PopulationId::NULL, IndividualId::NULL)
760+
.unwrap();
756761
tables.add_edge(0., 1000., 0, 1).unwrap();
757762
tables.add_edge(0., 1000., 0, 2).unwrap();
758763
tables.build_index().unwrap();
@@ -997,13 +1002,13 @@ mod test {
9971002
let mut tables = TableCollection::new(1000.).unwrap();
9981003

9991004
tables
1000-
.add_mutation(0, 0, crate::TSK_NULL, 1.123, Some(b"pajamas"))
1005+
.add_mutation(0, 0, MutationId::NULL, 1.123, Some(b"pajamas"))
10011006
.unwrap();
10021007
tables
1003-
.add_mutation(1, 1, crate::TSK_NULL, 2.123, None)
1008+
.add_mutation(1, 1, MutationId::NULL, 2.123, None)
10041009
.unwrap();
10051010
tables
1006-
.add_mutation(2, 2, crate::TSK_NULL, 3.123, Some(b"more pajamas"))
1011+
.add_mutation(2, 2, MutationId::NULL, 3.123, Some(b"more pajamas"))
10071012
.unwrap();
10081013
let mutations = tables.mutations();
10091014
assert!(close_enough(mutations.time(0).unwrap(), 1.123));
@@ -1012,9 +1017,9 @@ mod test {
10121017
assert_eq!(mutations.node(0).unwrap(), 0);
10131018
assert_eq!(mutations.node(1).unwrap(), 1);
10141019
assert_eq!(mutations.node(2).unwrap(), 2);
1015-
assert_eq!(mutations.parent(0).unwrap(), crate::TSK_NULL);
1016-
assert_eq!(mutations.parent(1).unwrap(), crate::TSK_NULL);
1017-
assert_eq!(mutations.parent(2).unwrap(), crate::TSK_NULL);
1020+
assert_eq!(mutations.parent(0).unwrap(), MutationId::NULL);
1021+
assert_eq!(mutations.parent(1).unwrap(), MutationId::NULL);
1022+
assert_eq!(mutations.parent(2).unwrap(), MutationId::NULL);
10181023
assert_eq!(mutations.derived_state(0).unwrap().unwrap(), b"pajamas");
10191024

10201025
if mutations.derived_state(1).unwrap().is_some() {
@@ -1099,7 +1104,7 @@ mod test {
10991104
.add_mutation_with_metadata(
11001105
0,
11011106
0,
1102-
crate::TSK_NULL,
1107+
MutationId::NULL,
11031108
1.123,
11041109
None,
11051110
Some(&F { x: -3, y: 666 }),
@@ -1126,15 +1131,15 @@ mod test {
11261131
.add_mutation_with_metadata(
11271132
0,
11281133
0,
1129-
crate::TSK_NULL,
1134+
MutationId::NULL,
11301135
1.123,
11311136
None,
11321137
Some(&F { x: -3, y: 666 }),
11331138
)
11341139
.unwrap();
11351140

11361141
tables
1137-
.add_mutation_with_metadata(1, 2, crate::TSK_NULL, 2.0, None, None)
1142+
.add_mutation_with_metadata(1, 2, MutationId::NULL, 2.0, None, None)
11381143
.unwrap();
11391144

11401145
let mut num_with_metadata = 0;
@@ -1167,7 +1172,7 @@ mod test {
11671172
assert_eq!(tables.populations().num_rows(), 1);
11681173

11691174
tables
1170-
.add_node(crate::TSK_NODE_IS_SAMPLE, 0.0, pop_id, crate::TSK_NULL)
1175+
.add_node(crate::TSK_NODE_IS_SAMPLE, 0.0, pop_id, IndividualId::NULL)
11711176
.unwrap();
11721177

11731178
match tables.nodes().row(NodeId::from(0)) {
@@ -1185,10 +1190,10 @@ mod test {
11851190
let mut tables = TableCollection::new(1000.).unwrap();
11861191
let pop_id = tables.add_population().unwrap();
11871192
tables
1188-
.add_node(crate::TSK_NODE_IS_SAMPLE, 0.0, pop_id, crate::TSK_NULL)
1193+
.add_node(crate::TSK_NODE_IS_SAMPLE, 0.0, pop_id, IndividualId::NULL)
11891194
.unwrap();
11901195
tables
1191-
.add_node(crate::TSK_NODE_IS_SAMPLE, 1.0, pop_id, crate::TSK_NULL)
1196+
.add_node(crate::TSK_NODE_IS_SAMPLE, 1.0, pop_id, IndividualId::NULL)
11921197
.unwrap();
11931198
tables.add_edge(0., tables.sequence_length(), 1, 0).unwrap();
11941199
tables
@@ -1286,7 +1291,7 @@ mod test_bad_metadata {
12861291
let mut tables = TableCollection::new(1.).unwrap();
12871292
let md = F { x: 1, y: 11 };
12881293
tables
1289-
.add_mutation_with_metadata(0, 0, crate::TSK_NULL, 0.0, None, Some(&md))
1294+
.add_mutation_with_metadata(0, 0, MutationId::NULL, 0.0, None, Some(&md))
12901295
.unwrap();
12911296
if tables.mutations().metadata::<Ff>(0.into()).is_ok() {
12921297
panic!("expected an error!!");
@@ -1307,26 +1312,26 @@ mod test_adding_node {
13071312
fn test_adding_node_without_metadata() {
13081313
let mut tables = make_empty_table_collection(10.);
13091314

1310-
match tables.add_node(0, 0.0, TSK_NULL, TSK_NULL) {
1315+
match tables.add_node(0, 0.0, PopulationId::NULL, IndividualId::NULL) {
13111316
Ok(NodeId(0)) => (),
13121317
_ => panic!("Expected NodeId(0)"),
13131318
};
13141319

13151320
let row = tables.nodes().row(NodeId::from(0)).unwrap();
13161321

13171322
assert_eq!(row.id, NodeId::from(0));
1318-
assert_eq!(row.population, PopulationId::from(TSK_NULL));
1319-
assert_eq!(row.individual, IndividualId::from(TSK_NULL));
1323+
assert_eq!(row.population, PopulationId::NULL);
1324+
assert_eq!(row.individual, IndividualId::NULL);
13201325
assert!(row.metadata.is_none());
13211326

13221327
let row_id = tables
1323-
.add_node(0, 0.0, PopulationId::from(2), TSK_NULL)
1328+
.add_node(0, 0.0, PopulationId::from(2), IndividualId::NULL)
13241329
.unwrap();
13251330

13261331
assert_eq!(tables.nodes().population(row_id).unwrap(), PopulationId(2));
13271332
assert_eq!(
13281333
tables.nodes().individual(row_id).unwrap(),
1329-
IndividualId(TSK_NULL)
1334+
IndividualId::NULL,
13301335
);
13311336
assert!(tables
13321337
.nodes()
@@ -1343,12 +1348,12 @@ mod test_adding_node {
13431348
.is_ok());
13441349

13451350
let row_id = tables
1346-
.add_node(0, 0.0, TSK_NULL, IndividualId::from(17))
1351+
.add_node(0, 0.0, PopulationId::NULL, IndividualId::from(17))
13471352
.unwrap();
13481353

13491354
assert_eq!(
13501355
tables.nodes().population(row_id).unwrap(),
1351-
PopulationId(TSK_NULL)
1356+
PopulationId::NULL,
13521357
);
13531358
assert_eq!(tables.nodes().individual(row_id).unwrap(), IndividualId(17));
13541359

@@ -1391,7 +1396,7 @@ mod test_adding_individual {
13911396
#[test]
13921397
fn test_adding_individual_without_metadata() {
13931398
let mut tables = make_empty_table_collection(10.);
1394-
match tables.add_individual(0, &[0., 0., 0.], &[TSK_NULL, TSK_NULL]) {
1399+
match tables.add_individual(0, &[0., 0., 0.], &[IndividualId::NULL, IndividualId::NULL]) {
13951400
Ok(IndividualId(0)) => (),
13961401
_ => panic!("Expected NodeId(0)"),
13971402
};
@@ -1403,7 +1408,7 @@ mod test_adding_individual {
14031408

14041409
assert_eq!(
14051410
row.parents,
1406-
Some(vec![IndividualId(TSK_NULL), IndividualId(TSK_NULL),])
1411+
Some(vec![IndividualId::NULL, IndividualId::NULL,])
14071412
);
14081413

14091414
// Empty slices are a thing, causing None to be in the rows.

0 commit comments

Comments
 (0)