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 label definition to add function for LabeledTask #340

Merged
merged 6 commits into from
Oct 30, 2023
Merged

Add label definition to add function for LabeledTask #340

merged 6 commits into from
Oct 30, 2023

Conversation

habibhaidari1
Copy link
Contributor

Description

I really liked the changes in #277 and would like to provide additional functionality. With this changes it is possible to add a label_definition and clear all labels from the task. I added the clear function to make the component in line with the other components like the attribute ruler e.g.
https://github.com/explosion/spaCy/blob/d717123819fb02cf81dcc26be305c0f9cd9893bf/spacy/pipeline/attributeruler.py#L102

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 all tests in tests and usage_examples/tests, and all new and existing tests passed. This includes
    • all external tests (i. e. pytest ran with --external)
    • all tests requiring a GPU (i. e. pytest ran with --gpu)
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

optional add label_definition
@habibhaidari1 habibhaidari1 changed the title Add label Add label definition to add function for LabeledTask Oct 25, 2023
@svlandeg svlandeg added enhancement Improvement of existing feature feat/task Feature: tasks labels Oct 26, 2023
@habibhaidari1
Copy link
Contributor Author

Sorry for the inconvenience. Pytest skipped the tests on my local machine because there was no API key stored there. I adjusted the test-case to catch non working label_definitions by adding a definition that slightly contradicts the label.

@rmitsch
Copy link
Collaborator

rmitsch commented Oct 27, 2023

Hi @habibhaidari1, thanks for your PR! We'll have a look and give some feedback shortly.

Copy link
Collaborator

@rmitsch rmitsch 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 overall, thanks! Can you run black on this? Ha, good timing.

@rmitsch rmitsch added the Test external Run external tests label Oct 30, 2023
@rmitsch rmitsch changed the base branch from develop to main October 30, 2023 12:17
@rmitsch
Copy link
Collaborator

rmitsch commented Oct 30, 2023

I'll go ahead and merge this, as the new test passes locally (and fails here due to the absence of the OpenAI API key in your repo).

Thanks for the contribution! 🎉 This will be part of the next patch release.

@rmitsch rmitsch merged commit e8cb182 into explosion:main Oct 30, 2023
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing feature feat/task Feature: tasks Test external Run external tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants