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

Displacy serve entity linking support without manual=True support. #9748

Merged

Conversation

narayanacharya6
Copy link
Contributor

The PR adds support for visualizing kb_id on entities when run via displacy.serve

Description

The PR adds support for visualizing kb_id on entities when run via displacy.serve. The recent changes from PR 9199 only address rendered html produced with the manual=True option set. I believe the same should also be afforded when passing Doc objects with entities that have the kb_id set.

For the kb_url to be set, one can pass the kb_url_format_template (can be named better?) in the options argument of displace.serve. If not set, kb_url defaults to # similar to the setting in the above linked PR. Please suggest if there is a better way to do this.

Types of change

Enhancement

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.

@svlandeg svlandeg added enhancement Feature requests and improvements feat / nel Feature: Named Entity linking feat / visualizers Feature: Built-in displaCy and other visualizers labels Nov 26, 2021
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.

Thanks for the PR, much appreciated! Also good to see an additional test for the new functionality. I had just a few small comments.

spacy/displacy/__init__.py Outdated Show resolved Hide resolved
spacy/displacy/__init__.py Outdated Show resolved Hide resolved
narayanacharya6 and others added 2 commits November 26, 2021 04:58
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
@svlandeg
Copy link
Member

Looks good to me, just want to run this by @ines as well :-)

narayanacharya6 and others added 2 commits November 26, 2021 23:56
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
Copy link
Member

@ines ines left a comment

Choose a reason for hiding this comment

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

Yes, looks good 🎉 And really appreciate the test and docs update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests and improvements feat / nel Feature: Named Entity linking feat / visualizers Feature: Built-in displaCy and other visualizers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants