metadata access counter; closes #3472

docs/metadata.md

@@ -153,6 +153,70 @@ time *units* of a tree sequence should be stored in the
 metadata. For example, we could set `tables.time_units = "generations"`.
 :::
 
+
+(sec_metadata_top_level_pitfalls)=
+
+### Pitfalls related to metadata
+
+Briefly, the pitfalls are:
+
+1. editing metadata directly can silently do nothing, and
+2. making repeated calls to top-level metadata will make your code very slow
+    if that metadata is large.
+
+One thing that it is important to know about metadata is that metadata access methods
+return a *copy* of the *decoded* information.
+So, for instance, each time you call
+```{code-cell}
+tables.metadata
+```
+(or ``ts.metadata``), it gives you a dictionary containing a *new copy* of the
+underlying information. So, this means that you cannot edit metadata
+by direct assignment. For instance, trying to change the metadata like this
+silently does nothing:
+```{code-cell}
+tables.metadata["taxonomy"]["subspecies"] = "lyrata"
+print(tables.metadata["taxonomy"]["subspecies"])
+```
+This is true for all types of metadata:
+for instance, editing `ts.mutation(0).metadata` will not change the metadata
+of the underlying mutation.
+See {ref}`sec_tutorial_metadata` for examples of modifying metadata in tables.
+To edit top-level metadata, first copy out the metadata, edit *that*,
+and then put it back:
+```{code-cell}
+md = tables.metadata
+md["taxonomy"]["species"] = "lyrata"
+tables.metadata = md
+```
+
+This fact about metadata can have important consequences for performance.
+For instance, if we'd like to convert all mutation times to generations,
+we should **definitely not** run
+``[mut.time / ts.metadata["generation_time"] for mut in ts.mutations()]``.
+This is because a new copy of the top-level metadata will be created
+for *every* mutation in the tree sequence.
+This may not be a big deal for some tree sequences,
+but this will take prohibitively long for tree sequences
+that have a large amount of information in top-level metadata
+(for instance, those produced by SLiM v6).
+Instead, it is good practice to make a copy of the top-level metadata,
+and use that copy henceforth, like so:
+```{code-cell}
+:tags: ["skip-execution"]
+top_md = ts.metadata
+mut_times = [
+    mut.time / top_md["generation_time"] for mut in ts.mutations()
+]
+```
+
+Because of this, tskit will produce a Warning if the top-level metadata
+is large (greater than 100Kb) and is accessed many times (more than 20 times).
+This behavior can be changed, by setting
+{data}`tskit.METADATA_ACCESS_WARNING_COUNT`
+or {data}`tskit.METADATA_ACCESS_WARNING_SIZE`.
+
+
 (sec_metadata_examples_reference_sequence)=
 
 ### Reference sequence
@@ -284,8 +348,9 @@ must be encoded and decoded. The C API does not do this, but the Python API will
 use the schema to decode the metadata to Python objects.
 The encoding for doing this is specified in the top-level schema property `codec`.
 Currently the Python API supports the `json` codec which encodes metadata as
-[JSON](https://www.json.org/json-en.html), and the `struct` codec which encodes
-metadata in an efficient schema-defined binary format using {func}`python:struct.pack` .
+[JSON](https://www.json.org/json-en.html), the `struct` codec which encodes
+metadata in an efficient schema-defined binary format using {func}`python:struct.pack`,
+and the `json+struct` codec which is a combination of the two.
 
 (sec_metadata_codecs_json)=
 
@@ -436,6 +501,12 @@ The supported numeric and boolean types are:
   - 8
 ```
 
+In addition to the `binaryFormat` encoding given in the table above,
+the `type` key must also be set to the appropriate value.
+For boolean values the `type` should be `boolean` (not bool);
+for integer binary formats it should be `integer` (not number),
+and for floating-point binary formats it should be `number`.
+
 When attempting to pack a non-integer using any of the integer conversion
 codes, if the non-integer has a `__index__` method then that method is
 called to convert the argument to an integer before packing.
@@ -602,7 +673,7 @@ into 8-byte alignment; and
 (7) the binary data.
 The JSON data is encoded as ASCII, without a null terminating byte,
 and the format of the binary data is specified using the "struct" portion
-of the metadata schema, described :ref:`above <sec_metadata_codecs_struct>`.
+of the metadata schema, described {ref}`above <sec_metadata_codecs_struct>`.
 
 (sec_metadata_schema_examples)=
 

docs/python-api.md

@@ -54,6 +54,7 @@ sequences.
   TreeSequence.discrete_genome
   TreeSequence.discrete_time
   TreeSequence.metadata
+  TreeSequence.metadata_size
   TreeSequence.metadata_schema
   TreeSequence.reference_sequence
 ```
@@ -723,6 +724,7 @@ Other properties
       TableCollection.nbytes
       TableCollection.table_name_map
       TableCollection.metadata
+      TableCollection.metadata_size
       TableCollection.metadata_bytes
       TableCollection.metadata_schema
       TableCollection.sequence_length

python/CHANGELOG.rst

@@ -10,6 +10,13 @@
 - The returned object from ``variant.counts()`` and ``variant.frequencies()``
   now stores alleles in the order defined in ``variant.alleles``.
   (:user:`hyanwong`, :pr:`3471`)
+- If top-level metadata is large, then repeatedly accessing it can be costly,
+  so now it throws a warning if the metadata is more than 200Kb and it is
+  accessed more than 20 times alerting the user to this possibility (both
+  constants are modifiable, however).
+  (:user:`petrelharp`, :issue:`3472`, :pr:`3475`)
+- TreeSequences and TableCollections now have a ``metadata_size`` property,
+  returning their size in bytes. (:user:`petrelharp`, :pr:`3475`)
 
 --------------------
 [1.0.3] - 2026-05-14

python/_tskitmodule.c

@@ -4050,6 +4050,19 @@ TableCollection_get_metadata(TableCollection *self, void *closure)
     return ret;
 }
 
+static PyObject *
+TableCollection_get_metadata_size(TableCollection *self, void *closure)
+{
+    PyObject *ret = NULL;
+
+    if (TableCollection_check_state(self) != 0) {
+        goto out;
+    }
+    ret = Py_BuildValue("n", (Py_ssize_t) self->tables->metadata_length);
+out:
+    return ret;
+}
+
 static int
 TableCollection_set_metadata(TableCollection *self, PyObject *arg, void *closure)
 {
@@ -5075,6 +5088,9 @@ static PyGetSetDef TableCollection_getsetters[] = {
         .get = (getter) TableCollection_get_metadata,
         .set = (setter) TableCollection_set_metadata,
         .doc = "The metadata." },
+    { .name = "metadata_size",
+        .get = (getter) TableCollection_get_metadata_size,
+        .doc = "Returns the size of the metadata, in bytes." },
     { .name = "metadata_schema",
         .get = (getter) TableCollection_get_metadata_schema,
         .set = (setter) TableCollection_set_metadata_schema,
@@ -5590,6 +5606,19 @@ TreeSequence_get_metadata(TreeSequence *self)
     return ret;
 }
 
+static PyObject *
+TreeSequence_get_metadata_size(TreeSequence *self)
+{
+    PyObject *ret = NULL;
+
+    if (TreeSequence_check_state(self) != 0) {
+        goto out;
+    }
+    ret = Py_BuildValue("n", (Py_ssize_t) self->tree_sequence->tables->metadata_length);
+out:
+    return ret;
+}
+
 static PyObject *
 TreeSequence_get_metadata_schema(TreeSequence *self)
 {
@@ -8759,6 +8788,10 @@ static PyMethodDef TreeSequence_methods[] = {
         .ml_meth = (PyCFunction) TreeSequence_get_metadata,
         .ml_flags = METH_NOARGS,
         .ml_doc = "Returns the metadata for the tree sequence" },
+    { .ml_name = "get_metadata_size",
+        .ml_meth = (PyCFunction) TreeSequence_get_metadata_size,
+        .ml_flags = METH_NOARGS,
+        .ml_doc = "Returns the size of the metadata, in bytes." },
     { .ml_name = "get_metadata_schema",
         .ml_meth = (PyCFunction) TreeSequence_get_metadata_schema,
         .ml_flags = METH_NOARGS,

python/tests/test_highlevel.py

@@ -42,7 +42,7 @@
 import unittest
 import uuid as _uuid
 import warnings
-from xml.etree import ElementTree
+from html.parser import HTMLParser
 
 import kastore
 import msprime
@@ -1988,7 +1988,7 @@ def test_load_tables(self, ts):
     def test_html_repr(self, ts):
         html = ts._repr_html_()
         # Parse to check valid
-        ElementTree.fromstring(html)
+        HTMLParser().feed(html)
         assert len(html) > 5000
         assert f"<tr><td>Trees</td><td>{ts.num_trees:,}</td></tr>" in html
         assert f"<tr><td>Time Units</td><td>{ts.time_units}</td></tr>" in html
@@ -3059,19 +3059,24 @@ def test_tree_sequence_metadata(self):
         tc = tskit.TableCollection(1)
         ts = tc.tree_sequence()
         assert ts.metadata == b""
+        assert ts.metadata_size == 0
         tc.metadata_schema = self.metadata_schema
         data = {
             "table": "tree-sequence",
             "string_prop": "stringy",
             "num_prop": 42,
         }
+        data_enc = self.metadata_schema.validate_and_encode_row(data)
         tc.metadata = data
         ts = tc.tree_sequence()
         assert ts.metadata == data
+        assert ts.metadata_size == len(data_enc)
         with pytest.raises(AttributeError):
             ts.metadata = {"should": "fail"}
         with pytest.raises(AttributeError):
             del ts.metadata
+        with pytest.raises(AttributeError):
+            ts.metadata_size = 0
 
     def test_tree_sequence_time_units(self):
         tc = tskit.TableCollection(1)
@@ -3712,7 +3717,7 @@ def test_str(self, ts_fixture):
     def test_html_repr(self, ts_fixture):
         html = ts_fixture.first()._repr_html_()
         # Parse to check valid
-        ElementTree.fromstring(html)
+        HTMLParser().feed(html)
         assert len(html) > 1900
         assert "<tr><td>Total Branch Length</td><td>" in html
 

python/tests/test_immutable_table_collection.py

@@ -30,6 +30,7 @@ def test_basic_properties_match(self, ts):
         assert mutable.file_uuid == immutable.file_uuid
         assert mutable.metadata_schema == immutable.metadata_schema
         assert mutable.metadata == immutable.metadata
+        assert mutable.metadata_size == immutable.metadata_size
         assert mutable.metadata_schema.encode_row(mutable.metadata) == bytes(
             immutable.metadata_bytes
         )

python/tests/test_metadata.py

@@ -2775,3 +2775,117 @@ def test_explicit_ordering(self):
 
         dtype = metadata.MetadataSchema(schema).numpy_dtype()
         assert dtype.names == ("id", "name", "age")
+
+
+class TestMetadataAccessCounter:
+    """
+    Test for top-level metadata multiple access warnings.
+    """
+
+    big_size = tskit.METADATA_ACCESS_WARNING_SIZE
+
+    def get_example(self, n, what):
+        # note this cannot be a fixture because the _metadata_access_counter
+        # will then persist across tests
+        t = tskit.TableCollection(sequence_length=1)
+        schema = metadata.MetadataSchema(
+            {
+                "codec": "json+struct",
+                "json": {
+                    "codec": "json",
+                    "type": "object",
+                    "properties": {
+                        "a": {"type": "string"},
+                    },
+                },
+                "struct": {
+                    "codec": "struct",
+                    "type": "object",
+                    "properties": {
+                        "x": {
+                            "type": "array",
+                            "arrayLengthFormat": "Q",
+                            "items": {"type": "integer", "binaryFormat": "q"},
+                        }
+                    },
+                },
+            }
+        )
+        md = {"a": "bcde", "x": list(range(n))}
+        t.metadata_schema = schema
+        t.metadata = md
+        if what != "tables":
+            t = t.tree_sequence()
+        if what == "immutable_tables":
+            t = t.tables
+        return t
+
+    def check_not_warns(self, t, num_checks=None):
+        if num_checks is None:
+            num_checks = tskit.METADATA_ACCESS_WARNING_COUNT + 5
+        md = t.metadata
+        for _ in range(num_checks):
+            x = t.metadata
+            assert md == x
+            x["a"] = "this should have no effect"
+
+    def check_warns(self, t):
+        md = t.metadata
+        # should warn after METADATA_ACCESS_WARNING_COUNT times
+        for _ in range(tskit.METADATA_ACCESS_WARNING_COUNT - 1):
+            x = t.metadata
+            assert md == x
+            x["a"] = "this should have no effect"
+        with pytest.warns(UserWarning, match="metadata is large"):
+            assert md == t.metadata
+        # and no more after that
+        for _ in range(5):
+            x = t.metadata
+            assert md == x
+            x["a"] = "this should have no effect"
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_warns(self, what):
+        t = self.get_example(self.big_size, what)
+        self.check_warns(t)
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_not_warns(self, what):
+        t = self.get_example(5, what)
+        self.check_not_warns(t)
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_change_threshhold_not_warns(self, what, monkeypatch):
+        orig_threshhold = tskit.METADATA_ACCESS_WARNING_COUNT
+        t = self.get_example(self.big_size, what)
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_COUNT", 1000)
+        self.check_not_warns(t, num_checks=orig_threshhold + 5)
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_change_threshhold_warns(self, what, monkeypatch):
+        t = self.get_example(self.big_size, what)
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_COUNT", 0)
+        with pytest.warns(UserWarning, match="metadata is large"):
+            _ = t.metadata
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_change_size_warns(self, what, monkeypatch):
+        t = self.get_example(5, what)
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_SIZE", 1)
+        self.check_warns(t)
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_change_size_not_warns(self, what, monkeypatch):
+        t = self.get_example(self.big_size, what)
+        # put down the threshhold so this doesn't take forever
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_SIZE", 2**32)
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_COUNT", 2)
+        self.check_not_warns(t)
+
+    def test_set_resets_counter(self, monkeypatch):
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_COUNT", 5)
+        t = self.get_example(self.big_size, "tables")
+        self.check_warns(t)
+        self.check_not_warns(t)
+        t.metadata = t.metadata
+        self.check_warns(t)