Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpanFinder into spaCy from experimental #12507

Merged
merged 58 commits into from
Jun 7, 2023

Conversation

kadarakos
Copy link
Contributor

Integrating the span_finder from spacy-experimental into spaCy.

Description

The SpanFinder is a component that suggest spans with a very simple tokenwise classifier: it decides for each token if it could be a start and/or end of a span. The code is largely just copied from spacy-experimental with the minimum amount of changes required to run the tests.

I added a couple of comments with the XXX annotation to ask a couple of initial questions about the parts I would consider changing after reading the code.

Types of change

Moving spacy-experimental component into spaCy.

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.

@kadarakos
Copy link
Contributor Author

Not sure why the project clone test fails in the Test Python39Linux?

@kadarakos kadarakos added feat / pipeline Feature: Processing pipeline and components feat / spanfinder Feature: Span Finder labels Apr 13, 2023
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
spacy/tests/pipeline/test_span_finder.py Outdated Show resolved Hide resolved
Co-authored-by: Adriane Boyd <adrianeboyd@gmail.com>
spacy/tests/pipeline/test_span_finder.py Outdated Show resolved Hide resolved
spacy/tests/pipeline/test_span_finder.py Outdated Show resolved Hide resolved
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I only had a few minor comments.

spacy/ml/models/span_finder.py Outdated Show resolved Hide resolved
spacy/pipeline/__init__.py Outdated Show resolved Hide resolved
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
spacy/pipeline/span_finder.py Outdated Show resolved Hide resolved
adrianeboyd and others added 2 commits June 7, 2023 11:41
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
@adrianeboyd
Copy link
Contributor

I'll go ahead and merge this so we can include it in a dev release. I'll be testing it a lot with several demo projects, so if anything crops up we can fix it before the v3.6.0 release.

@adrianeboyd adrianeboyd merged commit c003aac into explosion:master Jun 7, 2023
@svlandeg
Copy link
Member

svlandeg commented Jun 7, 2023

Thanks @kadarakos and @adrianeboyd for pushing this one over the line! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat / pipeline Feature: Processing pipeline and components feat / spanfinder Feature: Span Finder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants