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

Add apply CLI #11376

Merged
merged 48 commits into from
Dec 20, 2022
Merged

Add apply CLI #11376

merged 48 commits into from
Dec 20, 2022

Conversation

kadarakos
Copy link
Contributor

@kadarakos kadarakos commented Aug 24, 2022

New spacy cli command to make annotating new documents with the a trained pipeline convenient by just running a single command.

Description

Adds a new spacy cli command called apply that takes a model and a file or directory as input.
The file or the files in the directory should be of the following formats:

  1. .spacy file.
  2. .jsonl file where the provided --text-key is used to find the documents.
  3. Any other file found is considered a plain text file containing a single document.

Given the input file or directory apply generates a .spacy file containing the Doc objects produced by the model.

Types of change

New feature

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 kadarakos marked this pull request as draft August 24, 2022 15:29
@polm polm added enhancement Feature requests and improvements feat / cli Feature: Command-line interface labels Aug 25, 2022
@kadarakos
Copy link
Contributor Author

The way the data loading is done is with a bunch of nested try/except blocks, which I feel like is something we would like to avoid in general but it felt valid here.

Also this cli command is really short and simple compared to the rest so I feel like I must have missed a lot of things.

@kadarakos kadarakos marked this pull request as ready for review August 31, 2022 13:15
@adrianeboyd
Copy link
Contributor

I think it would make more sense to go by file ending here (maybe .spacy vs. not-.spacy) and take it from there?

@kadarakos
Copy link
Contributor Author

I think it would make more sense to go by file ending here (maybe .spacy vs. not-.spacy) and take it from there?

okok and then only the decode error remains in the tryexcept

spacy/cli/apply.py Outdated Show resolved Hide resolved
spacy/cli/apply.py Show resolved Hide resolved
spacy/cli/_util.py Outdated Show resolved Hide resolved
spacy/cli/convert.py Show resolved Hide resolved
@adrianeboyd adrianeboyd changed the base branch from v3.4.x to master October 21, 2022 06:28
spacy/tests/test_cli.py Outdated Show resolved Hide resolved
spacy/cli/apply.py Outdated Show resolved Hide resolved
spacy/cli/apply.py Outdated Show resolved Hide resolved
kadarakos and others added 6 commits November 23, 2022 09:58
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
spacy/cli/apply.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.

It looks like this PR should be good to merge as soon as the documentation is added?

spacy/cli/apply.py Outdated Show resolved Hide resolved
Copy link
Contributor

@polm polm 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 working on this, I think it's a really important feature to have! I'm sorry I didn't look at it early.

I have comments and docs fixes, but they're mostly pretty minor formatting stuff.

spacy/cli/_util.py Show resolved Hide resolved
spacy/cli/apply.py Outdated Show resolved Hide resolved
spacy/cli/apply.py Outdated Show resolved Hide resolved
spacy/cli/apply.py Outdated Show resolved Hide resolved
website/docs/api/cli.md Outdated Show resolved Hide resolved
website/docs/api/cli.md Outdated Show resolved Hide resolved
website/docs/api/cli.md Outdated Show resolved Hide resolved
kadarakos and others added 7 commits December 15, 2022 11:15
Co-authored-by: Paul O'Leary McCann <polm@dampfkraft.com>
Co-authored-by: Paul O'Leary McCann <polm@dampfkraft.com>
Co-authored-by: Paul O'Leary McCann <polm@dampfkraft.com>
Co-authored-by: Paul O'Leary McCann <polm@dampfkraft.com>
website/docs/api/cli.md 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.

Thanks again for all your work on this Akos, this will be a great new command to have!

@svlandeg svlandeg merged commit c223cd7 into explosion:master Dec 20, 2022
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 / cli Feature: Command-line interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants