[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Bugfixes and test for rehearse #10347

Merged
merged 9 commits into from
Feb 23, 2022
Merged

Bugfixes and test for rehearse #10347

merged 9 commits into from
Feb 23, 2022

Conversation

kadarakos
Copy link
Contributor
@kadarakos kadarakos commented Feb 21, 2022 •
edited
Loading

fixes #7161

@svlandeg svlandeg added training Training and updating models bug Bugs and behaviour differing from documentation labels Feb 22, 2022
svlandeg
svlandeg reviewed Feb 22, 2022
View reviewed changes
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 bug fix Ákos!

The rehearse functionality has been experimental for some time now, and it would be nice to get this into a better shape. This clear bug fix is certainly a good start.

Could you use the provided example code from issue 7161 to write a test that originally failed, and now succeeds with this bug fix?

@kadarakos
Copy link
Contributor Author

Could you use the provided example code from issue 7161 to write a test that originally failed, and now succeeds with this bug fix?

That's a very good idea, of course!

@kadarakos
Copy link
Contributor Author

Could you use the provided example code from issue 7161 to write a test that originally failed, and now succeeds with this bug fix?

The test uses the example from #7161 and now it triggers the rehearse for ner, tagger, parser and textcat_multilabel. As a result there are also bugfixes included for tagger and textcat_multilabel that make them run, but its not a fix in the sense that it makes them useful yet.

svlandeg
svlandeg approved these changes Feb 23, 2022
View reviewed changes
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, thanks for adding the tests! 🙏

kadarakos and others added 2 commits February 23, 2022 13:30
@kadarakos @svlandeg
Update spacy/tests/training/test_rehearse.py
373763b 
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
@kadarakos @svlandeg
Update spacy/tests/training/test_rehearse.py
9d3eaa2 
Co-authored-by: Sofie Van Landeghem <svlandeg@users.noreply.github.com>
@kadarakos kadarakos changed the title fixing argument order for rehearse Bugfixes and test for rehearse Feb 23, 2022
@honnibal
Copy link
Member

Looks good, and nice test!

@svlandeg svlandeg merged commit 249b971 into explosion:master Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svlandeg svlandeg svlandeg approved these changes

Assignees
No one assigned
Labels
bug Bugs and behaviour differing from documentation training Training and updating models
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nlp.rehearse error in spacy version 3.0.1
3 participants
@kadarakos @honnibal @svlandeg