Rename int and load flags to common namespace

[benjeffery]

Fixes #2219

[jeromekelleher]

LGTM.

I think we probably need to think through the flags semantics for table collection init/table init a little more clearly, but this is a good step.

[benjeffery]

Ha, I've actually created a collision here as all load flags are passed to init. I guess the only way to tidy this up nicely is to have a TSK_LOAD_INIT_* namespace, but it is a bit of a mouthful.

[benjeffery]

@jeromekelleher After playing around I've added to the PR to try and sort out the load/init flags, giving them a shared name space. It actually revealed an error where the tests were only working due to a collision.

[codecov]

Codecov Report

Merging #2224 (2ed6cd9) into main (2ed6cd9) will not change coverage.
The diff coverage is n/a.

❗ Current head 2ed6cd9 differs from pull request most recent head b1d3bfd. Consider uploading reports for the commit b1d3bfd to get more accurate results

@@           Coverage Diff           @@
##             main    #2224   +/-   ##
=======================================
  Coverage   93.30%   93.30%           
=======================================
  Files          27       27           
  Lines       26003    26003           
  Branches     1165     1165           
=======================================
  Hits        24262    24262           
  Misses       1711     1711           
  Partials       30       30           

Flag	Coverage Δ
c-tests	92.22% <0.00%> (ø)
lwt-tests	89.05% <0.00%> (ø)
python-c-tests	72.06% <0.00%> (ø)
python-tests	98.87% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ed6cd9...b1d3bfd. Read the comment docs.

[jeromekelleher]

I'm not sure this @benjeffery - what do you think of my suggestion of thinking of the flags to init/load as declaring properties of the thing (table,table collection) and namespace that way?

[jeromekelleher]

This one is different, in that it's only ever passed to tsk_treeseq_init(), right?

[jeromekelleher]

Could call TSK_TS_INIT_BUILD_INDEXES

[jeromekelleher]

LOAD_INIT seems a bit weird. Aren't these properties of the table collection/tables, which we're specifying at init time? So we could say something like

// This table has no metadata
TSK_TABLE_NO_METADATA (1 << 0)

// This table collection will have no reference sequence/tables/edge metadata
TSK_TC_NO_REFERENCE_SEQUENCE(1 << 0)
TSK_TC_NO_TABLES(1 << 1)
TSK_TC_NO_EDGE_METADATA (1 << 2)

[benjeffery]

I'm not sure this @benjeffery - what do you think of my suggestion of thinking of the flags to init/load as declaring properties of the thing (table,table collection) and namespace that way?

Yeah, will give that a try. Has to be better than this!

[benjeffery]

@jeromekelleher I think this makes more sense, have the shared namespace as a concept, but not all the same name.

[jeromekelleher]

Doesn't TSK_TS_INIT_BUILD_INDEXES belong in trees.h though, and live in a separate namespace? I don't think it interacts with the TableCollection of Table inits at all.

Ah, but SKIP_TABLES and SKIP_REFSEQ can also be passed to tsk_treeseq_load, right? So namespacing these as LOAD makes sense, maybe?

[benjeffery]

Yes, this is the issue - all flags that get passed to tsk_treeseq_load get passed to tsk_table_collection_load and then all passed on again to tsk_table_collection_init or tsk_table_collection_loadf_inited. So they all need to be in this block.

You're right though TSK_TS_INIT_BUILD_INDEXES is unrelated - got confused with tsk_tc_init. Will move that.

[benjeffery]

Will change the two "skips" to "loads" also as that makes more sense to me.

[benjeffery]

I also need to add a detailed change log here.

[jeromekelleher]

OK great, so we have some flags that we specifiy for load, and additionaly flags that can get passed through to init. I think that's making sense now and we just have to be clear on the documentation for this.