I'm revisiting this as (a) those two tskit issues have now been fixed and (b) I'm thinking about how tsdate adds metadata to nodes (specifically, the mean estimated time for a node and the variance associated with that time). At the moment this is simply dumped into the node metadata, which probably overwrites any existing values.

The need to add to the struct by tsdate makes me thing that perhaps the permissive JSON schema would be best for node metadata in tsinfer?

Alternatively, since tsdate is primarily going to be used by tsinfer, we could have some logic in tsdate that checks whether the schema matches tsinfer.node_schema and if so, swapps it for a schema which also includes space for the mean time and variance values. This would be more fiddly, but much more efficient in terms of space, I think.

P.s. just to note that sample data files don't contain metadata that can be set on nodes (only on individuals), so tsinfer is free to use whatever schema it wants for node metadata, without stomping on any user data.

I think the schema should look like this:

metadata.MetadataSchema(
    {
        "codec": "struct",
        "type": ["object", "null"],
        "properties": {
            "ancestor_data_id": {
                "description": "",
                "type": "integer",
                "binaryFormat": "i",
                "default": -1,
            },
            "sample_data_id": {
                "description": "Date of sample collection in ISO format",
                "type": "integer",
                "binaryFormat": "i",
                "default": -1,
            },
        },
        "required": ["ancestor_data_id", "sample_data_id"],
        "additionalProperties": False,
    }
)

I'm hesitant to use a struct schema to be honest, wouldn't JSON be a a lot simpler?

Not sure why we'd set defaults as well as marking them as required?

Looks like your descriptions are off?

I'm hesitant to use a struct schema to be honest, wouldn't JSON be a a lot simpler?

You said in the initial text of this issue "we should start with struct anyway just to see how it works out". Note that for big inferences, it might be quite tedious storing the JSON for each node (27 million ancestors in the unified genealogy, right?)

Not sure why we'd set defaults as well as marking them as required?

I thought they had to be required for a struct, but would take the default if not given? I probably misunderstood the meaning of the two or the interaction between them though.

Looks like your descriptions are off?

Oops, yes. Thanks for catching this.

Yes, that's true. 27M is quite a few, so the difference would be a few hundred megabytes. All right, I think that's a good justification for going with struct, let's try it out.

Given that this is not a metadata field controlled by the user, I agree that we should try struct first. tskit-dev/tskit#2129 might be a useful thing, which could be used if the user really wanted to change the type to JSON for later modification.

If we are using a schema, then we can also save this stuff under a "tsinfer" key, without taking up lots of space, which I think is a lot cleaner. So I propose using a schema like this:

schema = {
    "codec": "struct",
    "type": ["object", "null"],
    "properties": {
        "tsinfer": {
            "description": "Information about node identity from the tsinfer inference process",
            "type": "object",
            "default": {"ancestor_data_id": -1, "sample_data_id": -1},
            "properties": {
                "ancestor_data_id": {
                    "description":
                        "The corresponding ancestor ID "
                        "in the ancestors file created by the inference process, "
                        "or -1 if not applicable",
                    "type": "number",
                    "binaryFormat": "i",
                    "default": -1,
                },
                "sample_data_id": {
                    "description": 
                        "The corresponding sample ID "
                        "in the sample data file used for inference, "
                        "or -1 if not applicable",
                    "type": "number",
                    "binaryFormat": "i",
                    "default": -1,
                },
            },
        },
    },
    "additionalProperties": False,
}