Skip to content

add extra-phrase in rules #4518

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

alok1304
Copy link
Collaborator

@alok1304 alok1304 commented Aug 16, 2025

Reference #4515

I this PR, I am trying to add an extra-phrase maker in rules. This extra-phrase like [[6]] is inserted between the text Neither the name of and nor the names of its . I am doing using another repo i.e https://github.com/alok1304/mark-extra-phrase, i also explained in this repo readme how to use this also i added tests on this repo. Like How we add/mark extra-phrase.

I am doing three things, like where we insert extra-phrase:
Replace text between markers according to rules: (eg. between text Neither the name of and nor the names of its)
1. If extra_phrase already present then do nothing
2. If empty then insert extra_phrase
3. If words present then replace with extra_phrase

  • Inserting extra-phrase between the text Neither the name of and nor the names of its.
  • Deprecate rules if they are duplicates.
  • Add is_deprecated and replaced_by for duplicates rules.

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁
  • Updated documentation pages (if applicable)
  • Updated CHANGELOG.rst (if applicable)

Signed-off-by: Alok Kumar [email protected]

Signed-off-by: Alok Kumar <[email protected]>
@alok1304
Copy link
Collaborator Author

Many test cases are currently failing because the previous PR has not yet been merged. Several tests need to be updated, as the current results include false positives. Once the previous PR is merged, I will update the relevant test files accordingly to reflect the new changes.

Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alok1304 thanks for the PR, but these changes are not mergable.

See #4515 (comment) for more details on why you cannot:

  • replace named entitites with extra words markers
  • deprecate rules

See also alok1304/mark-extra-phrase#1 :)

this list of conditions and the following Disclaimer in the documentation
and/or other materials provided with the distribution.
.
. Neither the name of Agere Systems Inc. nor the names of the contributors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot replace these named entities with extra words markers, this has a huge performance hit. See #4515 (comment) for more details.

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.

2 participants