Repository: explosion/spaCy
Pull request: Fix: De/Serialize `SpanGroups` including the SpanGroup keys
## Description

Fixes #10685

This de/serializes `SpanGroups` objects as dicts, so that the keys corresponding to `SpanGroup`s are included.

This prevents the loss of `SpanGroup`s that have the same `.name` as other `SpanGroup`s within the same `SpanGroups` object (upon de/serialization of the `SpanGroups`).

### Notes / Questions

I figured I'd offer a solution to the problem. This pull request implements the "option 2" solution mentioned in #10685.

1. I was wondering if we should output a warning if we deserialized a `SpanGroups` that has more than 1 `SpanGroup` with the same `.name` (which would still occur when deserializing `SpanGroups` that were serialized in the old format). That's up to you, but in the 4th commit of this PR, I included what that might look like.
    * The warning text (and its identifer--currently `Warnings.W119`) are certainly open for changes, however.
        * I considered if it would be worth including the `Doc` object's text, as a way to identify what Doc the warning applies to, but from a glance at the existing warning messages, that kind of verbosity seemed a bit out of place. Maybe it's not, though.
    * Also: when deserializing, I'm not e.g. counting the number of times a given `.name` is duplicated; currently, I'm just checking, for each SpanGroup in the serialized `SpanGroups`, if its `.name` is already a key of the `SpanGroups`. The performance of that check seems pretty negligible. And my guess is that that additional information--the number of duplicated `.name`s--wouldn't be particularly useful for users.
2. I added tests in a new module, in `tests/serialize/test_serialize_spangroups.py`. There wasn't an existing test module that tested `SpanGroups` specifically (`tests/doc/test_span_group.py` focuses on `SpanGroup`), and it seemed like my tests would fit best within the `tests/serialize/` dir.
3. I maintained the existing definition of `SpanGroups._EMPTY_BYTES`--as a `msgpack_dumps([])`--even though I changed `SpanGroups` to be serialized as a `dict` (not a `list`). I don't think this harms anything, and for `SpanGroups` that are already serialized using the old format (as a list), this makes deserialization just a bit easier.
4. (I left a remaining "TODO" in the 4th commit:) I wonder if we should bother to check if a serialized `SpanGroups` is neither a `dict` nor a `list`, and if that's true (i.e. if it's an invalid serialization), then perhaps do something like raise an exception, and if so, what exception that should be. For now, I don't do any such checks/exception-raising.
5. Part of me would like to see more (preferably "real-world") deserialization tests for backwards-compatibility--if only for my own peace of mind. This pull request fixes a data loss issue, and I want to make sure that deserializing `SpanGroups` (e.g. via `Doc`s/`DocBin`s) with the new approach doesn't cause any new data loss. Let me know if you have any additional tests in mind (e.g. to be tested by the new `test_deserialize_spangroups_compat` function), or if things are looking fine as they are.

> As a side note, I have a general question about spaCy's test fixtures:
>     Can we expect them to stay defined as they are--forever?
> The main reason I ask is: at least in some cases, I'm inclined to define my own e.g. `Doc` object because I'm not sure if an existing Doc fixture might change in a way that would (ultimately) break (or not) a test. (For example, a Doc fixture might not define a `.spans` for the Doc--currently--but can I assume it will always stay that way? That is, once a fixture is created, is it set in stone?)

I'm open for discussion.

### Type of change
Bug fix

## Checklist
- [x] I confirm that I have the right to submit this contribution under the project's MIT license.
- [x] I ran the tests, and all new and existing tests passed.
- [x] My changes don't require a change to the documentation, or if they do, I've added all required information.

Problem statement:
SpanGroup()s of a SpanGroups() are lost upon de/serialization--if having the same .name
## Problem:

If a `SpanGroup.name` conflicts with another SpanGroup.name in a `SpanGroups` object--but the `SpanGroup`s are values of different keys of the SpanGroups object--then when the SpanGroups object is serialized and then deserialized, only 1 of the `SpanGroup`s will be present in the `SpanGroups` object.

This occurs when e.g. serializing/deserializing `Doc`s via `DocBin`--whether using DocBin's `get_docs()` or `to_bytes()`/`from_bytes()`.

A Doc's `SpanGroups` might be in such a state when (at least):
* a user creates a `SpanGroup` object and sets the `.name` attribute to the name of another SpanGroup within the Doc's `SpanGroups`; or
* a user changes an existing `SpanGroup`'s `.name` to the name of another SpanGroup within the Doc's `SpanGroups`.

### How to reproduce the behaviour:

(Executed with:)
* **spaCy version:** 3.2.4
* **Platform:** Linux-5.17.3-x86_64-with-glibc2.35
* **Python version:** 3.10.4

For example:

```python
import spacy
from spacy.tokens import SpanGroup, DocBin
nlp = spacy.blank("en")

doc = nlp("Will it blend?")
doc.spans['test'] = [doc[0:1]]
... # perhaps at some other point in code:
spangroup = SpanGroup(doc, name='test', spans=[doc[1:2]])
doc.spans['test2'] = spangroup

# Now choose your own adventure:
# ***A:
>>> doc.spans
{'test': [Will], 'test2': [it]}
>>> doc.spans.from_bytes(doc.spans.to_bytes())
{'test': [it]}

# ***B:
# (`doc.spans` is defined as it was prior to adventure "A":)
>>> db = DocBin(docs=[doc])
>>> redocs = list(db.get_docs(nlp.vocab))
>>> redocs[0].spans
{'test': [it]}

# and consequently:
>>> redb = db.from_bytes(db.to_bytes())
>>> redoc = list(redb.get_docs(nlp.vocab))[0]
>>> redoc.spans
{'test': [it]}

# which similarly occurs for `DocBin`'s `to_disk` and `from_disk`.
```

And it's also problematic if e.g. (as a special case):
```python
doc.spans['bad']  = SpanGroup(doc, spans=[doc[0:1]])
doc.spans['user'] = SpanGroup(doc, spans=[doc[1:2]])
```
since `SpanGroup`s without a given 'name' all default to the name "", and if/when they're set as values in a Doc's `SpanGroups`, only 1 will be loaded/set upon deserialization.

While I wouldn't expect most users to write code exactly like the simple/direct examples above, they do have the power to create "naked" `SpanGroup` objects (i.e. via `SpanGroup()`--outside of a `doc.spans`) and use/modify/assign them (to a `doc.spans`) as desired.

## Cause:

`SpanGroups.to_bytes` serializes its `SpanGroup`s as a list, and doesn't include their corresponding keys (in the `SpanGroups` container). When deserializing a `SpanGroups` object, each `SpanGroup` in the list is assigned as the value of the key corresponding to its `.name`. When multiple `SpanGroup`s have the same `.name`, only the last `SpanGroup` with that `.name` will be present in the `SpanGroups` object.

`DocBin.add()` serializes `SpanGroups` using their `.to_bytes()`, so de/serialization of `Doc`s this way inherits this behavior.

## Solution:

Checks are already in place for ensuring agreement between a `SpanGroup`'s `Doc` and a `SpanGroups`' `Doc`.
To prevent the above issues, we may want to at least have:
1. (both of the following:)
    1. `SpanGroups` ensure that no added `SpanGroup` has the same name as another SpanGroup in the SpanGroups object. This could be done by e.g.:
        1. iterate over the `SpanGroup`s, or cache their names (probably: neither of which you want to do); or:
        2. require that a `SpanGroup.name` matches the key it is the value of in `SpanGroups`; in other words, maintaining the invariant:
            ```python
            for name, span_group in doc.spans.items():
                assert span_group.name == name
            ```
    2. `SpanGroup`s prevent changing their `.name` (e.g. via attribute assignment)--*if the `SpanGroup` object is a value of its `Doc`'s `SpanGroups` object* (which `SpanGroup`s currently don't have to be)--:
        1. to a name of another SpanGroup in its same `SpanGroups` object; or:
        2. (if we required bullet "1.i.b" (noted above):) to any name other than the key it is the value of in its `Doc`'s `.spans`.
2. (or:) Change how `SpanGroups` are serialized/deserialized:
    * Include the key names.
    * though this will still allow multiple SpanGroup()s within a `SpanGroups` to have the same `.name`. (This may not be a problem, but I haven't done an exhaustive search of the codebase to see how `SpanGroup.name`s are used--other than for de/serialization.)

Those are the two general approaches that came to mind to address this, but let me know what you think.
I'm leaning towards option 2, because it's both simpler and I think more in line with how you're treating `SpanGroup`s currently. I don't think I'd have a problem implementing that solution, if you want to go forward with it or something like it.

Hints from issue discussion:
Thanks for the report, that does look like a bug. We'll look into it!

Commit message: Add more SpanGroups serialization tests

Test that serialized SpanGroups maintain their Span order

Patch to review:
diff --git a/spacy/errors.py b/spacy/errors.py
index bff8e7414fc..341923d70f9 100644
--- a/spacy/errors.py
+++ b/spacy/errors.py
@@ -200,6 +200,11 @@ class Warnings(metaclass=ErrorsWithCodes):
             "for the corpora used to train the language. Please check "
             "`nlp.meta[\"sources\"]` for any relevant links.")
     W119 = ("Overriding pipe name in `config` is not supported. Ignoring override '{name_in_config}'.")
+    W120 = ("Unable to load all spans in Doc.spans: more than one span group "
+            "with the name '{group_name}' was found in the saved spans data. "
+            "Only the last span group will be loaded under "
+            "Doc.spans['{group_name}']. Skipping span group with values: "
+            "{group_values}")
 
 
 class Errors(metaclass=ErrorsWithCodes):
diff --git a/spacy/tests/serialize/test_serialize_span_groups.py b/spacy/tests/serialize/test_serialize_span_groups.py
new file mode 100644
index 00000000000..d18dacff137
--- /dev/null
+++ b/spacy/tests/serialize/test_serialize_span_groups.py
@@ -0,0 +1,160 @@
+import pytest
+
+from spacy.tokens import Span, SpanGroup
+from spacy.tokens._dict_proxies import SpanGroups
+
+
+@pytest.mark.issue(10685)
+def test_issue10685(en_tokenizer):
+    """Test `SpanGroups` de/serialization"""
+    # Start with a Doc with no SpanGroups
+    doc = en_tokenizer("Will it blend?")
+
+    # Test empty `SpanGroups` de/serialization:
+    assert len(doc.spans) == 0
+    doc.spans.from_bytes(doc.spans.to_bytes())
+    assert len(doc.spans) == 0
+
+    # Test non-empty `SpanGroups` de/serialization:
+    doc.spans["test"] = SpanGroup(doc, name="test", spans=[doc[0:1]])
+    doc.spans["test2"] = SpanGroup(doc, name="test", spans=[doc[1:2]])
+
+    def assert_spangroups():
+        assert len(doc.spans) == 2
+        assert doc.spans["test"].name == "test"
+        assert doc.spans["test2"].name == "test"
+        assert list(doc.spans["test"]) == [doc[0:1]]
+        assert list(doc.spans["test2"]) == [doc[1:2]]
+
+    # Sanity check the currently-expected behavior
+    assert_spangroups()
+
+    # Now test serialization/deserialization:
+    doc.spans.from_bytes(doc.spans.to_bytes())
+
+    assert_spangroups()
+
+
+def test_span_groups_serialization_mismatches(en_tokenizer):
+    """Test the serialization of multiple mismatching `SpanGroups` keys and `SpanGroup.name`s"""
+    doc = en_tokenizer("How now, brown cow?")
+    # Some variety:
+    # 1 SpanGroup where its name matches its key
+    # 2 SpanGroups that have the same name--which is not a key
+    # 2 SpanGroups that have the same name--which is a key
+    # 1 SpanGroup that is a value for 2 different keys (where its name is a key)
+    # 1 SpanGroup that is a value for 2 different keys (where its name is not a key)
+    groups = doc.spans
+    groups["key1"] = SpanGroup(doc, name="key1", spans=[doc[0:1], doc[1:2]])
+    groups["key2"] = SpanGroup(doc, name="too", spans=[doc[3:4], doc[4:5]])
+    groups["key3"] = SpanGroup(doc, name="too", spans=[doc[1:2], doc[0:1]])
+    groups["key4"] = SpanGroup(doc, name="key4", spans=[doc[0:1]])
+    groups["key5"] = SpanGroup(doc, name="key4", spans=[doc[0:1]])
+    sg6 = SpanGroup(doc, name="key6", spans=[doc[0:1]])
+    groups["key6"] = sg6
+    groups["key7"] = sg6
+    sg8 = SpanGroup(doc, name="also", spans=[doc[1:2]])
+    groups["key8"] = sg8
+    groups["key9"] = sg8
+
+    regroups = SpanGroups(doc).from_bytes(groups.to_bytes())
+
+    # Assert regroups == groups
+    assert regroups.keys() == groups.keys()
+    for key, regroup in regroups.items():
+        # Assert regroup == groups[key]
+        assert regroup.name == groups[key].name
+        assert list(regroup) == list(groups[key])
+
+
+@pytest.mark.parametrize(
+    "spans_bytes,doc_text,expected_spangroups,expected_warning",
+    # The bytestrings below were generated from an earlier version of spaCy
+    # that serialized `SpanGroups` as a list of SpanGroup bytes (via SpanGroups.to_bytes).
+    # Comments preceding the bytestrings indicate from what Doc they were created.
+    [
+        # Empty SpanGroups:
+        (b"\x90", "", {}, False),
+        # doc = nlp("Will it blend?")
+        # doc.spans['test'] = SpanGroup(doc, name='test', spans=[doc[0:1]])
+        (
+            b"\x91\xc4C\x83\xa4name\xa4test\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x04",
+            "Will it blend?",
+            {"test": {"name": "test", "spans": [(0, 1)]}},
+            False,
+        ),
+        # doc = nlp("Will it blend?")
+        # doc.spans['test']  = SpanGroup(doc, name='test', spans=[doc[0:1]])
+        # doc.spans['test2'] = SpanGroup(doc, name='test', spans=[doc[1:2]])
+        (
+            b"\x92\xc4C\x83\xa4name\xa4test\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x04\xc4C\x83\xa4name\xa4test\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x05\x00\x00\x00\x07",
+            "Will it blend?",
+            # We expect only 1 SpanGroup to be in doc.spans in this example
+            # because there are 2 `SpanGroup`s that have the same .name. See #10685.
+            {"test": {"name": "test", "spans": [(1, 2)]}},
+            True,
+        ),
+        # doc = nlp('How now, brown cow?')
+        # doc.spans['key1'] = SpanGroup(doc, name='key1', spans=[doc[0:1], doc[1:2]])
+        # doc.spans['key2'] = SpanGroup(doc, name='too', spans=[doc[3:4], doc[4:5]])
+        # doc.spans['key3'] = SpanGroup(doc, name='too', spans=[doc[1:2], doc[0:1]])
+        # doc.spans['key4'] = SpanGroup(doc, name='key4', spans=[doc[0:1]])
+        # doc.spans['key5'] = SpanGroup(doc, name='key4', spans=[doc[0:1]])
+        (
+            b"\x95\xc4m\x83\xa4name\xa4key1\xa5attrs\x80\xa5spans\x92\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x04\x00\x00\x00\x07\xc4l\x83\xa4name\xa3too\xa5attrs\x80\xa5spans\x92\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x04\x00\x00\x00\t\x00\x00\x00\x0e\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x05\x00\x00\x00\x0f\x00\x00\x00\x12\xc4l\x83\xa4name\xa3too\xa5attrs\x80\xa5spans\x92\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x04\x00\x00\x00\x07\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\xc4C\x83\xa4name\xa4key4\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\xc4C\x83\xa4name\xa4key4\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03",
+            "How now, brown cow?",
+            {
+                "key1": {"name": "key1", "spans": [(0, 1), (1, 2)]},
+                "too": {"name": "too", "spans": [(1, 2), (0, 1)]},
+                "key4": {"name": "key4", "spans": [(0, 1)]},
+            },
+            True,
+        ),
+    ],
+)
+def test_deserialize_span_groups_compat(
+    en_tokenizer, spans_bytes, doc_text, expected_spangroups, expected_warning
+):
+    """Test backwards-compatibility of `SpanGroups` deserialization.
+    This uses serializations (bytes) from a prior version of spaCy.
+
+    spans_bytes (bytes): Serialized `SpanGroups` object.
+    doc_text (str): Doc text.
+    expected_spangroups (dict):
+        Dict mapping every expected (after deserialization) `SpanGroups` key
+        to a SpanGroup's "args", where a SpanGroup's args are given as a dict:
+          {"name": span_group.name,
+           "spans": [(span0.start, span0.end), ...]}
+    expected_warning (bool): Whether a warning is to be expected from .from_bytes()
+        --i.e. if more than 1 SpanGroup has the same .name within the `SpanGroups`.
+    """
+    doc = en_tokenizer(doc_text)
+
+    if expected_warning:
+        with pytest.warns(UserWarning):
+            doc.spans.from_bytes(spans_bytes)
+    else:
+        doc.spans.from_bytes(spans_bytes)
+
+    assert doc.spans.keys() == expected_spangroups.keys()
+    for name, spangroup_args in expected_spangroups.items():
+        assert doc.spans[name].name == spangroup_args["name"]
+        spans = [Span(doc, start, end) for start, end in spangroup_args["spans"]]
+        assert list(doc.spans[name]) == spans
+
+
+def test_span_groups_serialization(en_tokenizer):
+    doc = en_tokenizer("0 1 2 3 4 5 6")
+    span_groups = SpanGroups(doc)
+    spans = [doc[0:2], doc[1:3]]
+    sg1 = SpanGroup(doc, spans=spans)
+    span_groups["key1"] = sg1
+    span_groups["key2"] = sg1
+    span_groups["key3"] = []
+    reloaded_span_groups = SpanGroups(doc).from_bytes(span_groups.to_bytes())
+    assert span_groups.keys() == reloaded_span_groups.keys()
+    for key, value in span_groups.items():
+        assert all(
+            span == reloaded_span
+            for span, reloaded_span in zip(span_groups[key], reloaded_span_groups[key])
+        )
diff --git a/spacy/tokens/_dict_proxies.py b/spacy/tokens/_dict_proxies.py
index d9506769b13..9630da26120 100644
--- a/spacy/tokens/_dict_proxies.py
+++ b/spacy/tokens/_dict_proxies.py
@@ -1,10 +1,11 @@
-from typing import Iterable, Tuple, Union, Optional, TYPE_CHECKING
+from typing import Dict, Iterable, List, Tuple, Union, Optional, TYPE_CHECKING
+import warnings
 import weakref
 from collections import UserDict
 import srsly
 
 from .span_group import SpanGroup
-from ..errors import Errors
+from ..errors import Errors, Warnings
 
 
 if TYPE_CHECKING:
@@ -16,7 +17,7 @@
 # Why inherit from UserDict instead of dict here?
 # Well, the 'dict' class doesn't necessarily delegate everything nicely,
 # for performance reasons. The UserDict is slower but better behaved.
-# See https://treyhunner.com/2019/04/why-you-shouldnt-inherit-from-list-and-dict-in-python/0ww
+# See https://treyhunner.com/2019/04/why-you-shouldnt-inherit-from-list-and-dict-in-python/
 class SpanGroups(UserDict):
     """A dict-like proxy held by the Doc, to control access to span groups."""
 
@@ -53,20 +54,52 @@ def setdefault(self, key, default=None):
         return super().setdefault(key, default=default)
 
     def to_bytes(self) -> bytes:
-        # We don't need to serialize this as a dict, because the groups
-        # know their names.
+        # We serialize this as a dict in order to track the key(s) a SpanGroup
+        # is a value of (in a backward- and forward-compatible way), since
+        # a SpanGroup can have a key that doesn't match its `.name` (See #10685)
         if len(self) == 0:
             return self._EMPTY_BYTES
-        msg = [value.to_bytes() for value in self.values()]
+        msg: Dict[bytes, List[str]] = {}
+        for key, value in self.items():
+            msg.setdefault(value.to_bytes(), []).append(key)
         return srsly.msgpack_dumps(msg)
 
     def from_bytes(self, bytes_data: bytes) -> "SpanGroups":
-        msg = [] if bytes_data == self._EMPTY_BYTES else srsly.msgpack_loads(bytes_data)
+        # backwards-compatibility: bytes_data may be one of:
+        # b'', a serialized empty list, a serialized list of SpanGroup bytes
+        # or a serialized dict of SpanGroup bytes -> keys
+        msg = (
+            []
+            if not bytes_data or bytes_data == self._EMPTY_BYTES
+            else srsly.msgpack_loads(bytes_data)
+        )
         self.clear()
         doc = self._ensure_doc()
-        for value_bytes in msg:
-            group = SpanGroup(doc).from_bytes(value_bytes)
-            self[group.name] = group
+        if isinstance(msg, list):
+            # This is either the 1st version of `SpanGroups` serialization
+            # or there were no SpanGroups serialized
+            for value_bytes in msg:
+                group = SpanGroup(doc).from_bytes(value_bytes)
+                if group.name in self:
+                    # Display a warning if `msg` contains `SpanGroup`s
+                    # that have the same .name (attribute).
+                    # Because, for `SpanGroups` serialized as lists,
+                    # only 1 SpanGroup per .name is loaded. (See #10685)
+                    warnings.warn(
+                        Warnings.W120.format(
+                            group_name=group.name, group_values=self[group.name]
+                        )
+                    )
+                self[group.name] = group
+        else:
+            for value_bytes, keys in msg.items():
+                group = SpanGroup(doc).from_bytes(value_bytes)
+                # Deserialize `SpanGroup`s as copies because it's possible for two
+                # different `SpanGroup`s (pre-serialization) to have the same bytes
+                # (since they can have the same `.name`).
+                self[keys[0]] = group
+                for key in keys[1:]:
+                    self[key] = group.copy()
         return self
 
     def _ensure_doc(self) -> "Doc":
diff --git a/spacy/tokens/span_group.pyi b/spacy/tokens/span_group.pyi
index 26efc3ba09c..245eb4dbe35 100644
--- a/spacy/tokens/span_group.pyi
+++ b/spacy/tokens/span_group.pyi
@@ -24,3 +24,4 @@ class SpanGroup:
     def __getitem__(self, i: int) -> Span: ...
     def to_bytes(self) -> bytes: ...
     def from_bytes(self, bytes_data: bytes) -> SpanGroup: ...
+    def copy(self) -> SpanGroup: ...


Code review comments:
diff_hunk: @@ -0,0 +1,160 @@
+import pytest
+
+from spacy.tokens import Span, SpanGroup
+from spacy.tokens._dict_proxies import SpanGroups
+
+
+@pytest.mark.issue(10685)
+def test_issue10685(en_tokenizer):
+    """Test `SpanGroups` de/serialization"""
+    # Start with a Doc with no SpanGroups
+    doc = en_tokenizer("Will it blend?")
+
+    # Test empty `SpanGroups` de/serialization:
+    assert len(doc.spans) == 0
+    doc.spans.from_bytes(doc.spans.to_bytes())
+    assert len(doc.spans) == 0
+
+    # Test non-empty `SpanGroups` de/serialization:
+    doc.spans["test"] = SpanGroup(doc, name="test", spans=[doc[0:1]])
+    doc.spans["test2"] = SpanGroup(doc, name="test", spans=[doc[1:2]])
+
+    def assert_spangroups():
+        assert len(doc.spans) == 2
+        assert doc.spans["test"].name == "test"
+        assert doc.spans["test2"].name == "test"
+        assert list(doc.spans["test"]) == [doc[0:1]]
+        assert list(doc.spans["test2"]) == [doc[1:2]]
+
+    # Sanity check the currently-expected behavior
+    assert_spangroups()
+
+    # Now test serialization/deserialization:
+    doc.spans.from_bytes(doc.spans.to_bytes())
+
+    assert_spangroups()
+
+
+def test_span_groups_serialization_mismatches(en_tokenizer):
+    """Test the serialization of multiple mismatching `SpanGroups` keys and `SpanGroup.name`s"""
+    doc = en_tokenizer("How now, brown cow?")
+    # Some variety:
+    # 1 SpanGroup where its name matches its key
+    # 2 SpanGroups that have the same name--which is not a key
+    # 2 SpanGroups that have the same name--which is a key
+    # 1 SpanGroup that is a value for 2 different keys (where its name is a key)
+    # 1 SpanGroup that is a value for 2 different keys (where its name is not a key)
+    groups = doc.spans
+    groups["key1"] = SpanGroup(doc, name="key1", spans=[doc[0:1], doc[1:2]])
+    groups["key2"] = SpanGroup(doc, name="too", spans=[doc[3:4], doc[4:5]])
+    groups["key3"] = SpanGroup(doc, name="too", spans=[doc[1:2], doc[0:1]])
+    groups["key4"] = SpanGroup(doc, name="key4", spans=[doc[0:1]])
+    groups["key5"] = SpanGroup(doc, name="key4", spans=[doc[0:1]])
+    sg6 = SpanGroup(doc, name="key6", spans=[doc[0:1]])
+    groups["key6"] = sg6
+    groups["key7"] = sg6
+    sg8 = SpanGroup(doc, name="also", spans=[doc[1:2]])
+    groups["key8"] = sg8
+    groups["key9"] = sg8
+
+    regroups = SpanGroups(doc).from_bytes(groups.to_bytes())
+
+    # Assert regroups == groups
+    assert regroups.keys() == groups.keys()
+    for key, regroup in regroups.items():
+        # Assert regroup == groups[key]
+        assert regroup.name == groups[key].name
+        assert list(regroup) == list(groups[key])
+
+
+@pytest.mark.parametrize(
+    "spans_bytes,doc_text,expected_spangroups,expected_warning",
+    # The bytestrings below were generated from an earlier version of spaCy
+    # that serialized `SpanGroups` as a list of SpanGroup bytes (via SpanGroups.to_bytes).
+    # Comments preceding the bytestrings indicate from what Doc they were created.
+    [
+        # Empty SpanGroups:
+        (b"\x90", "", {}, False),
+        # doc = nlp("Will it blend?")
+        # doc.spans['test'] = SpanGroup(doc, name='test', spans=[doc[0:1]])
+        (
+            b"\x91\xc4C\x83\xa4name\xa4test\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x04",
+            "Will it blend?",
+            {"test": {"name": "test", "spans": [(0, 1)]}},
+            False,
+        ),
+        # doc = nlp("Will it blend?")
+        # doc.spans['test']  = SpanGroup(doc, name='test', spans=[doc[0:1]])
+        # doc.spans['test2'] = SpanGroup(doc, name='test', spans=[doc[1:2]])
+        (
+            b"\x92\xc4C\x83\xa4name\xa4test\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x04\xc4C\x83\xa4name\xa4test\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x05\x00\x00\x00\x07",
+            "Will it blend?",
+            # We expect only 1 SpanGroup to be in doc.spans in this example
+            # because there are 2 `SpanGroup`s that have the same .name. See #10685.
+            {"test": {"name": "test", "spans": [(1, 2)]}},
+            True,
+        ),
+        # doc = nlp('How now, brown cow?')
+        # doc.spans['key1'] = SpanGroup(doc, name='key1', spans=[doc[0:1], doc[1:2]])
+        # doc.spans['key2'] = SpanGroup(doc, name='too', spans=[doc[3:4], doc[4:5]])
+        # doc.spans['key3'] = SpanGroup(doc, name='too', spans=[doc[1:2], doc[0:1]])
+        # doc.spans['key4'] = SpanGroup(doc, name='key4', spans=[doc[0:1]])
+        # doc.spans['key5'] = SpanGroup(doc, name='key4', spans=[doc[0:1]])
+        (
+            b"\x95\xc4m\x83\xa4name\xa4key1\xa5attrs\x80\xa5spans\x92\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x04\x00\x00\x00\x07\xc4l\x83\xa4name\xa3too\xa5attrs\x80\xa5spans\x92\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x04\x00\x00\x00\t\x00\x00\x00\x0e\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x05\x00\x00\x00\x0f\x00\x00\x00\x12\xc4l\x83\xa4name\xa3too\xa5attrs\x80\xa5spans\x92\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x04\x00\x00\x00\x07\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\xc4C\x83\xa4name\xa4key4\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\xc4C\x83\xa4name\xa4key4\xa5attrs\x80\xa5spans\x91\xc4(\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03",
+            "How now, brown cow?",
+            {
+                "key1": {"name": "key1", "spans": [(0, 1), (1, 2)]},
+                "too": {"name": "too", "spans": [(1, 2), (0, 1)]},
+                "key4": {"name": "key4", "spans": [(0, 1)]},
+            },
+            True,
+        ),
+    ],
+)
+def test_deserialize_span_groups_compat(
+    en_tokenizer, spans_bytes, doc_text, expected_spangroups, expected_warning
+):
+    """Test backwards-compatibility of `SpanGroups` deserialization.
+    This uses serializations (bytes) from a prior version of spaCy.
+
+    spans_bytes (bytes): Serialized `SpanGroups` object.
+    doc_text (str): Doc text.
+    expected_spangroups (dict):
+        Dict mapping every expected (after deserialization) `SpanGroups` key
+        to a SpanGroup's "args", where a SpanGroup's args are given as a dict:
+          {"name": span_group.name,
+           "spans": [(span0.start, span0.end), ...]}
+    expected_warning (bool): Whether a warning is to be expected from .from_bytes()
+        --i.e. if more than 1 SpanGroup has the same .name within the `SpanGroups`.
+    """
+    doc = en_tokenizer(doc_text)
+
+    if expected_warning:
+        with pytest.warns(UserWarning):
+            doc.spans.from_bytes(spans_bytes)
+    else:
+        doc.spans.from_bytes(spans_bytes)
path: spacy/tests/serialize/test_serialize_span_groups.py
text: @user1:
Correct me if I'm wrong, but this doesn't catch the behaviour if `expected_warning` is `False` but the deserialization does in fact throw a warning, right?

(I wouldn't hold off on merging because of this though, it's just a very minor thing)

@author:
Oof. Good catch. I thought I had tested for that when I first wrote that test function... (Maybe my pytest config was turning those warnings into errors? Doubtful; it was probably just PEBKAC. Either way, it shouldn't happen again.)

You're right, and I've written a couple of lines to ensure that warnings aren't ~raised when we don't expect them.

@author:
(I pushed a commit with those changes to this same branch, but let me know if they should be elsewhere.)

@user2:
We have several places in the test suite that check for no warnings that should probably all be refactored together with a corresponding change in the pytest requirements. I've added a link to this discussion in the comments for our internal task so this can be done in the combination with the existing cases. Thanks for your attention to this!