TEST: Doc.ents as SpanGroup

[adrianeboyd]

Description

Overview of required changes to support SpanGroup rather than Tuple[Span]:

• implement slice for SpanGroup
• return SpanGroup for SpanGroup + x or x + SpanGroup rather than refusing to concatenate (currently without good error handling)

Side effects:

• if appending to Doc.ents, only Iterable[Span] is supported rather than other formats like raw entity tuples from matcher results, but you can still assign mixed data in any of the currently supported formats to Doc.ents
  • for the Matcher case, as_spans provides a good alternative

Types of change

?

Checklist

• I confirm that I have the right to submit this contribution under the project's MIT license.
• I ran the tests, and all new and existing tests passed.
• My changes don't require a change to the documentation, or if they do, I've added all required information.

[adrianeboyd]

I'm surprised that no one has complained about slicing span groups.

[adrianeboyd]

This is probably not the fastest way to do this.

[adrianeboyd]

And I suppose we could support step here, but I'm not sure why you'd want to do this.

[adrianeboyd]

And if we did this, we'd want to do it for all the Tuple[Span] cases from #12288.

[adrianeboyd]

So the current SpanGroup design with a weakref for the doc is basically a deal breaker for this proposal:

import spacy
nlp = spacy.blank("en")
ruler = nlp.add_pipe("entity_ruler")
ruler.add_patterns([{"label": "A", "pattern": "the"}])
print(nlp("the cat").ents)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "spacy/tokens/span_group.pyx", line 62, in spacy.tokens.span_group.SpanGroup.__repr__
    return str(list(self))
  File "spacy/tokens/span_group.pyx", line 179, in __iter__
    yield self[i]
  File "spacy/tokens/span_group.pyx", line 114, in spacy.tokens.span_group.SpanGroup.__getitem__
    return Span.cinit(self.doc, self.c[i])
  File "spacy/tokens/span_group.pyx", line 73, in spacy.tokens.span_group.SpanGroup.doc.__get__
    raise RuntimeError(Errors.E865)
RuntimeError: [E865] A SpanGroup is not functional after the corresponding Doc has been garbage collected. To keep using the spans, make sure that the corresponding Doc object is still available in the scope of your function.

I like it otherwise, but this (by itself) is definitely too breaking.

[honnibal]

I see the thinking here.

Is it too awkward to make SpanGroup take an argument that tells us whether to hold a real ref or a weakref?

[svlandeg]

Temporarily closing this PR as we currently don't have the bandwidth to finish this.