Skip to content
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

Add get censored words & censor middle only features #36

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

emso-c
Copy link

@emso-c emso-c commented Jul 15, 2021

Provided a solution for the issue #34. Sorry I kind of messed up with branches so this commit is merged with the other PR I created (#35).

Again, it doesn't break anything and can only be used if get_censored_words is True

It basically returns a Tuple of (str, list) with the str being the original censored text and the list being the list of censored words.

Usage:

from better_profanity import profanity

if __name__ == "__main__":
    profanity.load_censor_words()
    
    text = "test fucking shit"
    censored_text, censored_words = profanity.censor(text, get_censored_words=True)
    print(censored_words)
    # ['fucking', 'shit']

Copy link
Owner

@snguyenthanh snguyenthanh left a comment

Choose a reason for hiding this comment

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

It would be great if you could write some tests for the functions you added as well.

@@ -53,7 +54,7 @@ def __init__(self, words=None):

## PUBLIC ##

def censor(self, text, censor_char="*"):
def censor(self, text, censor_char="*", middle_only=False, get_censored_words=False):
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be better to have a separate get_censored_words(text) function, as it is not obviously clear the returned result of censor(get_censored_words=True) is. You could share some/most of the code with censor function.

Copy link
Author

Choose a reason for hiding this comment

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

Oh I thought it would be better to not repeat the same code, but it's true that it would potentially cause confusion. I'll work on it.

Copy link
Owner

Choose a reason for hiding this comment

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

That's great. You are right that it would be better to not repeat the same code. But we could also create a function that is used by both censor and get_censored_words to determine which words are profane.

@@ -18,8 +18,14 @@ def read_wordlist(filename: str):
yield row


def get_replacement_for_swear_word(censor_char):
return censor_char * 4
def get_replacement_for_swear_word(censor_char, n=4):
Copy link
Owner

Choose a reason for hiding this comment

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

It's just my personal preference: could you replace n with a more detailed variable ? It's quite unclear what n is when we call get_replacement_for_swear_word("-", n=2).

Copy link
Author

Choose a reason for hiding this comment

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

Sure.

@emso-c emso-c changed the title Add get censored words feature Add get censored words & censor middle only features Jul 20, 2021
@emso-c
Copy link
Author

emso-c commented Jul 20, 2021

Separated the functions as you said but while writing unit tests and testing edge cases, I just realized that the current _hide_swear_words function merges multiple swear words into one when they're next to each other. Though both functionalities work well for single unseparated swear words, they don't behave well in these kind of situations.

Example with get_censored_words:

bad_text = "Dude, I hate shit. Fuck bullshit."
profanity.get_censored_words(bad_text)
>>>['shit', 'bullshit']
# It completely ignored "Fuck" since they're merged

bad_text = "That wh0re gave m3 a very good H@nD j0b."
profanity.get_censored_words(bad_text)
>>>['wh0re', 'H@nD']
# It didn't include "j0b" since they're separated with space

Example with middle_only (same issues):

bad_text = "Dude, I hate shit. Fuck bullshit."
profanity.censor(bad_text, middle_only=True)
>>>"Dude, I hate s**t b******t."
# It completely ignored "Fuck" since they're merged

bad_text = "That wh0re gave m3 a very good H@nD j0b."
profanity.censor(bad_text, middle_only=True)
>>>"That w***e gave m3 a very good H**D."
# It didn't include "j0b" since they're separated with space


To solve that, I simply put a check before merging swear words (only merge if: not (get_censored_words or middle_only)). The results are better and they pass all other unit tests, but bit of a coverage your method provided has disappeared. Which means it'll detect less swear words and it might result in inconsistencies.

Example with get_censored_words:

bad_text = "Dude, I hate shit. Fuck bullshit."
profanity.get_censored_words(bad_text)
>>>['shit', 'Fuck', 'bullshit']

bad_text = "That wh0re gave m3 a very good H@nD j0b."
profanity.get_censored_words(bad_text)
>>>['wh0re']
# It didn't include "H@nD j0b"

Example with middle_only (same issues):

bad_text = "Dude, I hate shit. Fuck bullshit."
profanity.censor(bad_text, middle_only=True)
>>>"Dude, I hate s**t. F**k b******t."


bad_text = "That wh0re gave m3 a very good H@nD j0b."
profanity.censor(bad_text, middle_only=True)
>>>"That w***e gave m3 a very good H@nD j0b."
# It didn't include "H@nD j0b"


Maybe we should just follow this method and warn users of these possible issues? I think it's a pretty mild edge case anyway, but it's up to you.

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