metadata access counter; closes #3472

docs/metadata.md

@@ -153,6 +153,69 @@ 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.
+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, using
+{data}`tskit.METADATA_ACCESS_WARNING_THRESHHOLD`
+and {data}`tskit.METADATA_ACCESS_WARNING_SIZE`.
+
+
 (sec_metadata_examples_reference_sequence)=
 
 ### Reference sequence
@@ -284,8 +347,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 +500,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 +672,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)=
 

python/CHANGELOG.rst

@@ -10,6 +10,11 @@
 - 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`)
 
 --------------------
 [1.0.3] - 2026-05-14

python/tests/test_metadata.py

@@ -2775,3 +2775,110 @@ 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_THRESHHOLD + 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):
+        # should warn after METADATA_ACCESS_WARNING_THRESHHOLD times
+        # and no more after that
+        md = t.metadata
+        for _ in range(tskit.METADATA_ACCESS_WARNING_THRESHHOLD - 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
+        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_THRESHHOLD
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_THRESHHOLD", 1000)
+        t = self.get_example(self.big_size, what)
+        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):
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_THRESHHOLD", 0)
+        t = self.get_example(self.big_size, what)
+        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):
+        # note this must be set before initialization!
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_SIZE", 1)
+        t = self.get_example(5, what)
+        self.check_warns(t)
+
+    @pytest.mark.parametrize("what", ["tables", "ts", "immutable_tables"])
+    def test_change_size_not_warns(self, what, monkeypatch):
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_SIZE", 2**32)
+        t = self.get_example(self.big_size, what)
+        # put down the threshhold so this doesn't take forever
+        monkeypatch.setattr(tskit, "METADATA_ACCESS_WARNING_THRESHHOLD", 2)
+        self.check_not_warns(t)

python/tskit/__init__.py

@@ -70,6 +70,16 @@
     "provenances",
 ]
 
+#: Threshholds for warning about multiple top-level metadata use.
+#: A warning will be thrown if top-level metadata is larger than "size" bytes and is
+#: accessed more that "threshhold" times.  To disable, set threshhold to a large number,
+#: for instance: ``tskit.METADATA_ACCESS_WARNING_THRESHOLD = 2**32``.
+METADATA_ACCESS_WARNING_THRESHHOLD = 20
+#: The minimum size of top-level metadata before a multiple access warning
+#: is produced. The threshhold may be set at any time, but for changing this variable
+#: to take effect, it must be set before initialization of the TreeSequences
+#: or TableCollections.
+METADATA_ACCESS_WARNING_SIZE = 200_000
 
 from tskit.provenance import __version__  # NOQA
 from tskit.provenance import validate_provenance  # NOQA

python/tskit/tables.py

@@ -3058,6 +3058,10 @@ def __init__(self, sequence_length=0, *, ll_tables=None):
         self._mutations = MutationTable(ll_table=self._ll_tables.mutations)
         self._populations = PopulationTable(ll_table=self._ll_tables.populations)
         self._provenances = ProvenanceTable(ll_table=self._ll_tables.provenances)
+        if len(self._ll_tables.metadata) > tskit.METADATA_ACCESS_WARNING_SIZE:
+            self._metadata_access_counter = 0
+        else:
+            self._metadata_access_counter = None
 
     @property
     def individuals(self) -> IndividualTable:
@@ -3196,6 +3200,32 @@ def has_reference_sequence(self):
         """
         return bool(self._ll_tables.has_reference_sequence())
 
+    @property
+    def metadata(self):
+        if self._metadata_access_counter is not None:
+            self._metadata_access_counter += 1
+            if self._metadata_access_counter > tskit.METADATA_ACCESS_WARNING_THRESHHOLD:
+                warnings.warn(
+                    "It looks like you're making repeated calls to "
+                    "<table collection>.metadata. "
+                    "If metadata is large, this can slow scripts down considerably. "
+                    "Instead, assign metadata to an object and use that, e.g.: "
+                    "tables_metadata = tables.metadata",
+                    UserWarning,
+                    stacklevel=2,
+                )
+                self._metadata_access_counter = None
+        return self.metadata_schema.decode_row(self.metadata_bytes)
+
+    @metadata.setter
+    def metadata(self, metadata):
+        encoded = self.metadata_schema.validate_and_encode_row(metadata)
+        if len(encoded) > tskit.METADATA_ACCESS_WARNING_SIZE:
+            self._metadata_access_counter = 0
+        else:
+            self._metadata_access_counter = None
+        self._ll_object.metadata = encoded
+
     @property
     def reference_sequence(self):
         """
@@ -4640,6 +4670,10 @@ def __init__(self, ll_tree_sequence):
         self.mutations = ImmutableMutationTable(ll_tree_sequence)
         self.populations = ImmutablePopulationTable(ll_tree_sequence)
         self.provenances = ImmutableProvenanceTable(ll_tree_sequence)
+        if len(ll_tree_sequence.get_metadata()) > tskit.METADATA_ACCESS_WARNING_SIZE:
+            self._metadata_access_counter = 0
+        else:
+            self._metadata_access_counter = None
         object.__setattr__(self, "_initialised", True)
 
     @property
@@ -4664,6 +4698,22 @@ def metadata_schema(self):
 
     @property
     def metadata(self):
+        if self._metadata_access_counter is not None:
+            # can't set things directly on this immutable class
+            object.__setattr__(
+                self, "_metadata_access_counter", self._metadata_access_counter + 1
+            )
+            if self._metadata_access_counter > tskit.METADATA_ACCESS_WARNING_THRESHHOLD:
+                warnings.warn(
+                    "It looks like you're making repeated calls to "
+                    "<table collection>.metadata. "
+                    "If metadata is large, this can slow scripts down considerably. "
+                    "Instead, assign metadata to an object and use that, e.g.: "
+                    "tables_metadata = tables.metadata",
+                    UserWarning,
+                    stacklevel=2,
+                )
+                object.__setattr__(self, "_metadata_access_counter", None)
         return self.metadata_schema.decode_row(self.metadata_bytes)
 
     @property

python/tskit/trees.py

@@ -27,7 +27,6 @@
 from __future__ import annotations
 
 import base64
-import builtins
 import collections
 import concurrent.futures
 import functools
@@ -118,7 +117,7 @@ def store_tree_sequence(cls):
 
     # Intercept the init to record the tree_sequence
     def new_init(self, *args, tree_sequence=None, **kwargs):
-        builtins.object.__setattr__(self, "_tree_sequence", tree_sequence)
+        object.__setattr__(self, "_tree_sequence", tree_sequence)
         wrapped_init(self, *args, **kwargs)
 
     cls.__init__ = new_init
@@ -4166,6 +4165,10 @@ def __init__(self, ll_tree_sequence):
             if not name.startswith("_")
         }
         self._table_metadata_schemas = TableMetadataSchemas(**metadata_schema_instances)
+        if len(ll_tree_sequence.get_metadata()) > tskit.METADATA_ACCESS_WARNING_SIZE:
+            self._metadata_access_counter = 0
+        else:
+            self._metadata_access_counter = None
         self._individuals_time = None
         self._individuals_population = None
         self._individuals_location = None
@@ -4613,7 +4616,27 @@ def get_sequence_length(self):
     def metadata(self) -> Any:
         """
         The decoded metadata for this TreeSequence.
-        """
+
+        This is a *property*, so each time you call TreeSequence.metadata,
+        the underlying metadata is decoded, and a new copy of the result is
+        returned (usually as a dictionary). So, calling this many times is
+        inefficient, and for repeated use the metadata should be stored. For
+        instance, instead of ``[ts.metadata['t'] - m.time for m in ts.mutations()]``,
+        do ``md = ts.metadata; [md['t'] - m.time for m in ts.mutations()]``.
+        """
+        if self._metadata_access_counter is not None:
+            self._metadata_access_counter += 1
+            if self._metadata_access_counter > tskit.METADATA_ACCESS_WARNING_THRESHHOLD:
+                warnings.warn(
+                    "It looks like you're making repeated calls to "
+                    "<tree sequence>.metadata. "
+                    "If metadata is large, this can slow scripts down considerably. "
+                    "Instead, assign metadata to an object and use that, e.g.: "
+                    "ts_metadata = ts.metadata",
+                    UserWarning,
+                    stacklevel=2,
+                )
+                self._metadata_access_counter = None
         return self.metadata_schema.decode_row(self._ll_tree_sequence.get_metadata())
 
     @property
@@ -7503,7 +7526,7 @@ def subset(
         Returns a tree sequence containing only information directly
         referencing the provided list of nodes to retain.  The result will
         retain only the nodes whose IDs are listed in ``nodes``, only edges for
-        which both parent and child are in ``nodes```, only mutations whose
+        which both parent and child are in ``nodes``, only mutations whose
         node is in ``nodes``, and only individuals that are referred to by one
         of the retained nodes.  Note that this does *not* retain
         the ancestry of these nodes - for that, see :meth:`.simplify`.