-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Tests for CLI app - init config
generates train
-able config
#12173
Tests for CLI app - init config
generates train
-able config
#12173
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice test! If I understand correctly, currently the test checks whether init-config
generates a runnable config for a pipeline with a single component? If so, should it also check combinations of components?
I think that's do-able, it'll explode the parameter space for this test though. Are there any sets of components that have interaction effects - i.e. it's not just adding a new section for |
Hmm, the first thing that came to my mind is the |
Oh yeah, the I think that's probably worth setting up another test with these combinations, also because the example data we put into it will have to change so that all elements needed for the pipeline are annotated - right now I only provide attributes for each specific component. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good and should be ready to merge. I'll just run the slow tests once more on this PR (they succeed locally) and will merge when green.
@explosion-bot please test_slow |
URL: https://buildkite.com/explosion-ai/spacy-slow-tests/builds/427 |
The Before the last commit on this PR,
which means that logger levels are sometimes set to INFO even if they were DEBUG before. In
BUT that won't actually do anything if the main logger is set to INFO. Because To fix this, in the last commit we're setting the logger level only to |
@explosion-bot please test_slow |
URL: https://buildkite.com/explosion-ai/spacy-slow-tests/builds/430 |
Adds a CLI app test that the
init config
command generates configs that are usable bytrain
for each possible component.Created as draft to collect feedback on this approach.
Description
The test is parameterized by each component's name and a set of example data that is serialized to a
DocBin
within the test (so that it can be used bytrain
). My intent with this structure is to avoid writing redundant code with a test for each component, which would really balloon the length of this file - but we can also take that more verbose approach as well well if we think that's better, I'm not totally convinced on the parameterized version myself. We'd probably want to do some custom formatting of the input parameters as well because they're also quite lengthy.I marked the test as
slow
because some components take a while to train (specifically theparser
). Locally this test ran in:Types of change
Tests
Checklist