Skip to content

Remove stale disabled flag when removing a pipeline component - #14070

Open
davrens101 wants to merge 1 commit into
explosion:masterfrom
davrens101:fix-remove-disabled-pipe
Open

davrens101 wants to merge 1 commit into
explosion:masterfrom
davrens101:fix-remove-disabled-pipe

Conversation

@davrens101

Copy link
Copy Markdown

Fixes #14069.

Language.remove_pipe pops the component from _components before checking name in self.disabled. The disabled property only lists disabled names that are still present in _components, so the check is always false for the component being removed and its name is left behind in _disabled. Adding a new component with the same name then silently inherits the stale disabled state.

This replaces the check with self._disabled.discard(name), which removes the entry unconditionally (and is a no-op when the component wasn't disabled).

Before:

>>> nlp = spacy.blank("en")
>>> nlp.add_pipe("sentencizer"); nlp.disable_pipe("sentencizer"); nlp.remove_pipe("sentencizer")
>>> nlp.add_pipe("sentencizer")
>>> nlp.disabled
['sentencizer']
>>> nlp.pipe_names
[]
>>> nlp("One. Two.").has_annotation("SENT_START")
False

After:

>>> nlp.add_pipe("sentencizer")  # after the same remove/re-add sequence
>>> nlp.disabled
[]
>>> nlp.pipe_names
['sentencizer']
>>> nlp("One. Two.").has_annotation("SENT_START")
True

Added a regression test in spacy/tests/pipeline/test_pipe_methods.py that asserts the disabled flag is cleared on removal and that a re-added component runs. Verified the new test fails on the unpatched language.py and passes with the fix; the full test_pipe_methods.py suite (46 tests) passes.

Checklist

  • I have submitted the spaCy Contributor Agreement.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't affect the spacy CLI.

Signed-off-by: davrens101 <davrens101@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Removing a disabled component leaves its name disabled when added again

1 participant