Repository: explosion/spaCy
Pull request: Remove side effects from Doc.__init__()
## Description
The existing version of `Doc.__init__()` changes `sent_starts` and `ents` lists passed as parameters. This PR removes these side effects.

Closes [#11499.](https://github.com/explosion/spaCy/issues/11499)

### Types of change
Bugfix

## Checklist
<!--- Before you submit the PR, go over this checklist and make sure you can
tick off all the boxes. [] -> [x] -->
- [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:
Doc.init changes input to is_sent_start to List[int]
Hey,

I just stumbled accross the following behaviour, when creating a Doc manually. Is this intended behaviour?

## How to reproduce the behaviour
    import spacy
    from spacy.tokens import Doc
    nlp = spacy.blank("xx")
    words_1 = ["This","is","the","first","sentence","."]
    spaces_1 =[True,True,True,True,False,True]
    sent_starts_1 = [True,False,False,False,False,False]
    test_doc = Doc( vocab=nlp.vocab,words=words_1, spaces=spaces_1, sent_starts=sent_starts_1)
    print(sent_starts_1)
    #which prints [1, -1, -1, -1, -1, -1]

## Your Environment
## Info about spaCy

- **spaCy version:** 3.4.1
- **Platform:** Linux-5.10.16.3-microsoft-standard-WSL2-x86_64-with-glibc2.29
- **Python version:** 3.8.10

Hints from issue discussion:
Thanks for letting us know about this, I'll have a look at fixing it.

Commit message: Remove side effects from Doc.__init__()

Patch to review:
diff --git a/spacy/tests/doc/test_doc_api.py b/spacy/tests/doc/test_doc_api.py
index a64ab2ba89b..0cfea24a782 100644
--- a/spacy/tests/doc/test_doc_api.py
+++ b/spacy/tests/doc/test_doc_api.py
@@ -82,6 +82,28 @@ def test_issue2396(en_vocab):
     assert (span.get_lca_matrix() == matrix).all()
 
 
+@pytest.mark.issue(11499)
+def test_issue11499(en_vocab):
+    print(en_vocab.strings[0])
+    words = ["A", "sentence"]
+    spaces = [True, False]
+    heads = [1, None]
+    deps = [None, None]
+    ents = ["B-TYPE1", "I-TYPE2"]
+    sent_starts = [True, False]
+    Doc(
+        vocab=en_vocab,
+        words=words,
+        spaces=spaces,
+        heads=heads,
+        deps=deps,
+        ents=ents,
+        sent_starts=sent_starts,
+    )
+    assert ents == ["B-TYPE1", "I-TYPE2"]
+    assert sent_starts == [True, False]
+
+
 @pytest.mark.parametrize("text", ["-0.23", "+123,456", "±1"])
 @pytest.mark.parametrize("lang_cls", [English, MultiLanguage])
 @pytest.mark.issue(2782)
diff --git a/spacy/tokens/doc.pyx b/spacy/tokens/doc.pyx
index 7ba9a334184..4f10f151cad 100644
--- a/spacy/tokens/doc.pyx
+++ b/spacy/tokens/doc.pyx
@@ -285,14 +285,15 @@ cdef class Doc:
             heads = [0] * len(deps)
         if heads and not deps:
             raise ValueError(Errors.E1017)
+        _sent_starts: Optional[List[int]] = [] if sent_starts is not None else None
         if sent_starts is not None:
             for i in range(len(sent_starts)):
-                if sent_starts[i] is True:
-                    sent_starts[i] = 1
-                elif sent_starts[i] is False:
-                    sent_starts[i] = -1
-                elif sent_starts[i] is None or sent_starts[i] not in [-1, 0, 1]:
-                    sent_starts[i] = 0
+                if sent_starts[i] is True or sent_starts[i] == 1:
+                    _sent_starts.append(1)
+                elif sent_starts[i] is False or sent_starts[i] == -1:
+                    _sent_starts.append(-1)
+                else:
+                    _sent_starts.append(0)
         if pos is not None:
             for pp in set(pos):
                 if pp not in parts_of_speech.IDS:
@@ -300,12 +301,11 @@ cdef class Doc:
         ent_iobs = None
         ent_types = None
         if ents is not None:
+            ents = [ent if ent != "" else None for ent in ents]
             iob_strings = Token.iob_strings()
             # make valid IOB2 out of IOB1 or IOB2
             for i, ent in enumerate(ents):
-                if ent is "":
-                    ents[i] = None
-                elif ent is not None and not isinstance(ent, str):
+                if ent is not None and not isinstance(ent, str):
                     raise ValueError(Errors.E177.format(tag=ent))
                 if i < len(ents) - 1:
                     # OI -> OB
@@ -338,14 +338,14 @@ cdef class Doc:
                     ent_types.append(ent_type)
         headings = []
         values = []
-        annotations = [pos, heads, deps, lemmas, tags, morphs, sent_starts, ent_iobs, ent_types]
+        annotations = [pos, heads, deps, lemmas, tags, morphs, _sent_starts, ent_iobs, ent_types]
         possible_headings = [POS, HEAD, DEP, LEMMA, TAG, MORPH, SENT_START, ENT_IOB, ENT_TYPE]
         for a, annot in enumerate(annotations):
             if annot is not None:
                 if len(annot) != len(words):
                     raise ValueError(Errors.E189)
                 headings.append(possible_headings[a])
-                if annot is not heads and annot is not sent_starts and annot is not ent_iobs:
+                if annot is not heads and annot is not _sent_starts and annot is not ent_iobs:
                     values.extend(annot)
         for value in values:
             if value is not None:
@@ -358,7 +358,7 @@ cdef class Doc:
             j = 0
             for annot in annotations:
                 if annot:
-                    if annot is heads or annot is sent_starts or annot is ent_iobs:
+                    if annot is heads or annot is _sent_starts or annot is ent_iobs:
                         for i in range(len(words)):
                             if attrs.ndim == 1:
                                 attrs[i] = annot[i]


Code review comments:
diff_hunk: @@ -285,27 +285,27 @@ cdef class Doc:
             heads = [0] * len(deps)
         if heads and not deps:
             raise ValueError(Errors.E1017)
+        _sent_starts: Optional[List[int]] = [] if sent_starts is not None else None
path: spacy/tokens/doc.pyx
text: @user1:
Based on the processing below, it looks like `int` values are supported, so I think the original type should be corrected to `Optional[List[Union[bool, int, None]]]` and then I think you can just make a copy and not need to modify the other processing steps:

```python
sent_starts = list(sent_starts) if sent_starts is not None else None
```

@author:
It would certainly be good to keep `sent_starts` rather than `_sent_starts` lower down in the method, but is performance not a concern at all? That way we make an additional copy of the list.

@user1:
I don't think there's going to be a big difference between creating a copy vs. creating a new list variable. I think it's much simpler to just create a copy to avoid this side effect.