Improvements to remove_word + new method: remove_word_list#18
Improvements to remove_word + new method: remove_word_list#18RicoViking9000 wants to merge 43 commits intoareebbeigh:masterfrom
Conversation
New features
* The remove_word() method now can remove words from an instance's custom_censor_list if it's used
* New argument - Universal, default True: if True, remove word from wherever it is; namely extra_censor_list if applicable
* New method - remove_word_list() - remove a list of words, following similar format as remove_word()
Updated remove_word():
*If _custom_censor_list is used, remove words from there instead of returning error for value not in _censor_list
*New arg - anywhere: If true, remove words from _extra_censor_list if word is there (checks there first)
*Updated docstring as appropriate
New method remove_word_list():
*Similar to remove_word(), but pass a list instead of string
*If _custom_censor_list is used, remove words from there instead of returning error for value not in _censor_list
*New arg - anywhere: If true, remove words from _extra_censor_list if word is there (checks there first)
*Updated docstring as appropriate
New method remove_word_list():
*Similar to remove_word(), but pass a list instead of string
areebbeigh
left a comment
areebbeigh
left a comment
There was a problem hiding this comment.
The improvement ideas are good. A few changes and this PR should be good to merge.
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
updated docstring for remove_words()
optimized and compressed remove_words() code
Fixed indentation
fixed docstring indentation
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
|
Okay, now this needs tests. If you're not familiar with writing tests take a look at how https://github.com/areebbeigh/profanityfilter/blob/master/tests/test_profanity.py is written. That's the file you'll be working with. Also, take a look at |
|
I will get to that this weekend, I appreciate the information |
Updated remove_word() for new features - removing from _extra_censor_list Added remove_words()
fixed assertion errors
|
I also have code locally (off github) for get_word_boundaries() and set_word_boundaries(bool), I can add them here if you prefer; I'm using these methods for my application, but you're welcome to keep it locked to instantiation only, totally up to you |
areebbeigh
left a comment
areebbeigh
left a comment
There was a problem hiding this comment.
Tests are failing https://travis-ci.org/areebbeigh/profanityfilter/jobs/497660502
|
Anywhere defaults to True in the method signatures, but I will redo my testing. I appreciate your patience with me here. |
|
In that case, test if anywhere=True does remove from both the lists and False doesn't. :) |
program now properly tests using both values of the 'anywhere' bool for remove_word() and remove_words()
fixed misspelling of 'chocolate'
I think I finally realize how the testing works here
|
Should I remove the lines that cause ValueErrors due to a certain word not being in the profanity list for remove_word()/remove_words(), and replace them with lines that pass the TravisCi testing? |
areebbeigh
left a comment
areebbeigh
left a comment
There was a problem hiding this comment.
When the word to be removed isn't found, we want an error to be raised. This behavior is expected and can be tested with https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertRaises
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
Co-Authored-By: RicoViking9000 <38385704+RicoViking9000@users.noreply.github.com>
|
I will add code to the test file this weekend |
Added assertRaise testing
it's assert RAISES, not RAISE
fixed improper uses of "raises" testing
Following programming logic and the way the remove words method was coded, the program would remove words individually, and would stop at the point it encountered the error, not before. Fixed incorrect assertion test due to this
|
This code has passed the tests |
|
2020 review: This should be added back into the main repo, since the current master has not been updated since December 2018. |
|
Hi @RicoViking9000 , Are you able to rebase this fork so we can have github actions against it. Regards, |
4 participants
When I was using your (wonderful by the way) profanity filter module, I did happen to realize that there was a limitation for my use - there was no way to remove words from the extra_censor_list except for using the restore_words() method and re-defining the extra_censor_list without the word to remove.
I have forked and modified the remove_word() method to better suit my needs and possibly the needs of other people. If you see this as something other people might make use of, you're welcome to merge.
In summary, I added an argument to remove_word (defaults to True), which, if True, checks if the word is in extra_censor_list before moving on to the other lists. In addition, previously there was no easy way to remove a word from the custom_censor_list without redefining, so I modified the method to, by default, do that if a custom_censor_list is used.
I also added a new method - remove_word_list, which makes sense to me as define_words takes a list, so now we have a method that takes a list and essentially does the same thing as remove_word(), but with a list.
Finally, I updated the Docstring slightly to be in conjunction with my edits.
The most efficient and working changes are the latest commit (60b1ad5)