Repository: explosion/spaCy
Pull request: Ignore overrides for pipe names in config argument
## Goals

Resolves https://github.com/explosion/spaCy/issues/10766 by ignoring name overrides in pipes' `config` argument.

<!--- Provide a general summary of your changes in the title. -->

## Description
<!--- Use this section to describe your changes. If your changes required
testing, include information about the testing environment and the tests you
ran. If your test fixes a bug reported in an issue, don't forget to include the
issue number. If your PR is still a work in progress, that's totally fine – just
include a note to let us know. -->
Removes overrides for `name` in pipes' `config` argument. Issues a warning if this override is set. Extends existing test to ensure pipe name isn't changed when overriden in `config`.

### Types of change
<!-- What type of change does your PR cover? Is it a bug fix, an enhancement
or new feature, or a change to the documentation? -->
Bug fix/prevention.

## Checklist
<!--- Before you submit the PR, go over this checklist and make sure you can
tick off all the boxes. [] -> [x] -->
- [x] I confirm that I have the right to submit this contribution under the project's MIT license.
- [x] I ran the tests, and all new and existing tests passed.
- [x] My changes don't require a change to the documentation, or if they do, I've added all required information.


Problem statement:
Overriding `name` argument of Pipe via config has inconsistent behavior once model is serialized and loaded back.
The `name` parameter of a `Pipe` is different once model is serialized to disk and loaded back if overridden via the config. While it is useless in most use case to specify a name for a pipe and then override it via the config as well but I believe the name should remain the same when a model is built and when it is loaded from disk using the config. 

(This is a weird one and probably not something that deserves attention but raising an issue in any case)

## How to reproduce the behaviour
```
import spacy
from spacy.language import Language
@Language.factory("example.Factory.v1")
class ExampleFactory:
    def __init__(self, nlp, name):
        self.nlp =  nlp
        self.name = name
        
    def __call__(self, doc):
        print(self.name)
        return doc
    
nlp = spacy.blank("en")
pipe = nlp.add_pipe("example.Factory.v1", name="arg_name", config={"name": "config_arg_name"})
content = "This is an example sentence!"
doc = nlp(content) # <-- Prints config_arg_name
nlp.to_disk("/tmp/example_model/")
loaded_nlp = spacy.load("/tmp/example_model/")
loaded_doc = loaded_nlp(content) # <-- Prints arg_name, I would expect it to be same as config_arg_name
```

## Your Environment
## Info about spaCy

- **spaCy version:** 3.0.6
- **Platform:** macOS-10.16-x86_64-i386-64bit
- **Python version:** 3.8.8
- **Pipelines:** en_core_web_lg (3.0.0), en_ud_model_trf (2.0.0), en_ud_model_lg (2.0.0), en_ud_model_sm (2.0.0), en_ud_model_md (2.0.0), en_core_web_md (3.0.0), en_core_web_sm (3.0.0), en_core_web_trf (3.0.0)

Hints from issue discussion:
Thanks for reporting this, @narayanacharya6! Looks like a bug indeed (also in the current v3.3). We'll look into it.

Commit message: Pipe name override in config: added check with warning, added removal of name override from config, extended tests.

Patch to review:
diff --git a/spacy/errors.py b/spacy/errors.py
index b01afcb802f..bff8e7414fc 100644
--- a/spacy/errors.py
+++ b/spacy/errors.py
@@ -199,6 +199,7 @@ class Warnings(metaclass=ErrorsWithCodes):
     W118 = ("Term '{term}' not found in glossary. It may however be explained in documentation "
             "for the corpora used to train the language. Please check "
             "`nlp.meta[\"sources\"]` for any relevant links.")
+    W119 = ("Overriding pipe name in `config` is not supported. Ignoring override '{name_in_config}'.")
 
 
 class Errors(metaclass=ErrorsWithCodes):
diff --git a/spacy/language.py b/spacy/language.py
index bab403f0eb4..faca1f25883 100644
--- a/spacy/language.py
+++ b/spacy/language.py
@@ -774,6 +774,9 @@ def add_pipe(
         name = name if name is not None else factory_name
         if name in self.component_names:
             raise ValueError(Errors.E007.format(name=name, opts=self.component_names))
+        # Overriding pipe name in the config is not supported and will be ignored.
+        if "name" in config:
+            warnings.warn(Warnings.W119.format(name_in_config=config.pop("name")))
         if source is not None:
             # We're loading the component from a model. After loading the
             # component, we know its real factory name
diff --git a/spacy/tests/pipeline/test_pipe_factories.py b/spacy/tests/pipeline/test_pipe_factories.py
index 4128e2a48c6..9e55ebd17c9 100644
--- a/spacy/tests/pipeline/test_pipe_factories.py
+++ b/spacy/tests/pipeline/test_pipe_factories.py
@@ -119,6 +119,7 @@ def __init__(
             self.value1 = value1
             self.value2 = value2
             self.is_base = True
+            self.name = name
 
         def __call__(self, doc: Doc) -> Doc:
             return doc
@@ -141,12 +142,13 @@ def __call__(self, doc: Doc) -> Doc:
         nlp.add_pipe(name)
     with pytest.raises(ConfigValidationError):  # invalid config
         nlp.add_pipe(name, config={"value1": "10", "value2": "hello"})
-    nlp.add_pipe(name, config={"value1": 10, "value2": "hello"})
+    nlp.add_pipe(name, config={"value1": 10, "value2": "hello", "name": "wrong_name"})
     pipe = nlp.get_pipe(name)
     assert isinstance(pipe.nlp, Language)
     assert pipe.value1 == 10
     assert pipe.value2 == "hello"
     assert pipe.is_base is True
+    assert pipe.name == name
 
     nlp_en = English()
     with pytest.raises(ConfigValidationError):  # invalid config


Code review comments:
diff_hunk: @@ -141,12 +142,13 @@ def __call__(self, doc: Doc) -> Doc:
         nlp.add_pipe(name)
     with pytest.raises(ConfigValidationError):  # invalid config
         nlp.add_pipe(name, config={"value1": "10", "value2": "hello"})
-    nlp.add_pipe(name, config={"value1": 10, "value2": "hello"})
+    nlp.add_pipe(name, config={"value1": 10, "value2": "hello", "name": "wrong_name"})
path: spacy/tests/pipeline/test_pipe_factories.py
text: @user1:
```suggestion
    with pytest.warns(UserWarning):
        nlp.add_pipe(name, config={"value1": 10, "value2": "hello", "name": "wrong_name"})
```