From 89018142489386cbff5e5146b5997e4ed6315aec Mon Sep 17 00:00:00 2001 From: ines Date: Tue, 30 Jan 2018 15:43:03 +0100 Subject: [PATCH 1/5] Improve error handling if pipeline component is not callable (resolves #1911) Also add help message if user accidentally calls nlp.add_pipe() with a string of a built-in component name. --- spacy/language.py | 8 ++++++++ spacy/tests/pipeline/test_pipe_methods.py | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/spacy/language.py b/spacy/language.py index ec0c5d68f..d4204294e 100644 --- a/spacy/language.py +++ b/spacy/language.py @@ -236,6 +236,14 @@ class Language(object): >>> nlp.add_pipe(component, before='ner') >>> nlp.add_pipe(component, name='custom_name', last=True) """ + if not hasattr(component, '__call__'): + msg = ("Not a valid pipeline component. Expected callable, but " + "got {}. ".format(repr(component))) + if component in self.factories.keys(): + msg += ("If you meant to add a built-in component, use " + "create_pipe: nlp.add_pipe(nlp.create_pipe('{}'))" + .format(component)) + raise ValueError(msg) if name is None: if hasattr(component, 'name'): name = component.name diff --git a/spacy/tests/pipeline/test_pipe_methods.py b/spacy/tests/pipeline/test_pipe_methods.py index c0165d004..6c12e162e 100644 --- a/spacy/tests/pipeline/test_pipe_methods.py +++ b/spacy/tests/pipeline/test_pipe_methods.py @@ -107,3 +107,9 @@ def test_add_lots_of_pipes(nlp, n_pipes): for i in range(n_pipes): nlp.add_pipe(lambda doc: doc, name='pipe_%d' % i) assert len(nlp.pipe_names) == n_pipes + + +@pytest.mark.parametrize('component', ['ner', {'hello': 'world'}]) +def test_raise_for_invalid_components(nlp, component): + with pytest.raises(ValueError): + nlp.add_pipe(component) From ce10d320c42a3b081856f9255f382cfb9820c5a7 Mon Sep 17 00:00:00 2001 From: ines Date: Tue, 30 Jan 2018 16:09:37 +0100 Subject: [PATCH 2/5] Fix component check in self.factories (see #1911) --- spacy/language.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spacy/language.py b/spacy/language.py index d4204294e..1a60b2542 100644 --- a/spacy/language.py +++ b/spacy/language.py @@ -239,7 +239,7 @@ class Language(object): if not hasattr(component, '__call__'): msg = ("Not a valid pipeline component. Expected callable, but " "got {}. ".format(repr(component))) - if component in self.factories.keys(): + if component in self.factories: msg += ("If you meant to add a built-in component, use " "create_pipe: nlp.add_pipe(nlp.create_pipe('{}'))" .format(component)) From 404682369986262ee6975e7341a2ec01c8151c7c Mon Sep 17 00:00:00 2001 From: ines Date: Tue, 30 Jan 2018 16:29:07 +0100 Subject: [PATCH 3/5] Only check component in factories if string (see #1911) --- spacy/language.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spacy/language.py b/spacy/language.py index 1a60b2542..ae62f918a 100644 --- a/spacy/language.py +++ b/spacy/language.py @@ -239,7 +239,7 @@ class Language(object): if not hasattr(component, '__call__'): msg = ("Not a valid pipeline component. Expected callable, but " "got {}. ".format(repr(component))) - if component in self.factories: + if isinstance(component, basestring_) and component in self.factories: msg += ("If you meant to add a built-in component, use " "create_pipe: nlp.add_pipe(nlp.create_pipe('{}'))" .format(component)) From a0b912c528c699ff346a89880672f7b3352d1248 Mon Sep 17 00:00:00 2001 From: Hassan Shamim Date: Tue, 30 Jan 2018 15:01:01 -0800 Subject: [PATCH 4/5] fix broken link to test suite models --- website/usage/_install/_instructions.jade | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/usage/_install/_instructions.jade b/website/usage/_install/_instructions.jade index 66c60da32..aeab67d2f 100644 --- a/website/usage/_install/_instructions.jade +++ b/website/usage/_install/_instructions.jade @@ -238,7 +238,7 @@ p python -m pytest <spacy-directory> --models --en # basic and English model tests +infobox("Note on model tests", "⚠️") - | The test suite specifies a #[+a(gh("spacy", "tests/conftest.py")) list of models] + | The test suite specifies a #[+a(gh("spacy", "spacy/tests/conftest.py")) list of models] | to run the tests on. If a model is not installed, the tests will be | skipped. If all models are installed, the respective tests will run once | for each model. The easiest way to find out which models and model From 3c1fb9d02d55066ce7dd30e554204c57cdd04710 Mon Sep 17 00:00:00 2001 From: ines Date: Wed, 31 Jan 2018 22:06:28 +0100 Subject: [PATCH 5/5] Make validate command fail more gracefully if version not found Mostly relevant during develoment when working with .dev versions --- spacy/cli/validate.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spacy/cli/validate.py b/spacy/cli/validate.py index f96f7683e..b83753509 100644 --- a/spacy/cli/validate.py +++ b/spacy/cli/validate.py @@ -20,13 +20,16 @@ def validate(): prints("Couldn't fetch compatibility table.", title="Server error (%d)" % r.status_code, exits=1) compat = r.json()['spacy'] + current_compat = compat.get(about.__version__) + if not current_compat: + prints(about.__compatibility__, exits=1, + title="Can't find spaCy v{} in compatibility table" + .format(about.__version__)) all_models = set() for spacy_v, models in dict(compat).items(): all_models.update(models.keys()) for model, model_vs in models.items(): compat[spacy_v][model] = [reformat_version(v) for v in model_vs] - - current_compat = compat[about.__version__] model_links = get_model_links(current_compat) model_pkgs = get_model_pkgs(current_compat, all_models) incompat_links = {l for l, d in model_links.items() if not d['compat']}