pdrhlik / sweary

R package that collects swear words from different languages.
MIT License
19 stars 8 forks source link

Some functions to go along the db #25

Open ColinFay opened 6 years ago

ColinFay commented 6 years ago

Here are some funs to play with the db:

sweary_is("shit", "en")
[1] TRUE

vec <- c("Yeah, nigga, we're still fucking with you, Still waters run deep ,Still Snoop Dogg and D.R.E. '99, nigga, guess who's back; Still, still doin' that shit, Andre? Oh for sho' Yeah! Check me out")

sweary_n(vec, "en")
[1] 3

sweary_pct(vec, "en")
[1] 8.823529

sweary_any(vec, "en")
[1] TRUE

sweary_censor(vec, "en")
[1] "Yeah, ***, we're still ***ing with you, Still waters run deep ,Still Snoop Dogg and D.R.E. '99, ***, guess who's back; Still, still doin' that ***, Andre? Oh for sho' Yeah! Check me out"
pdrhlik commented 6 years ago

Hi @ColinFay! Thanks for the PR, it's awesome!

You added some things that were definitely on the way. You just pushed it in the right direction :-)

I have a few comments on some of the commits.

  1. How exactly is devtools_history.R used? I haven't seen it before and I don't know its purpose although I suspect it.
  2. The main 5 functions are great. I'm only not sure about the naming convention you started. I'd probably prefer to be more explicit. Mainly because it might be used in long pipes with different commands. If we are explicit at the cost of longer function names, the code will be more easily readable. So I suggest naming it like n_swearwords instead of sweary_n.
  3. Could you add newlines to every file?
  4. Could you remove the empty lines between function documentations and bodies?
  5. attempt looks great. I'd like the package to be with as less dependencies as possible. I am using quite a lot of packages and none of them forced me to install attempt yet :-) Could you persuade me?
  6. Your tests don't pass. Check the travis ouput. Sneak peek at the result is below.
    en_swear_words <- get_swearwords("en")
    Error in .x %in% swear_lang : object 'swear_lang' not found
    Calls: get_swearwords ... check_is_lang -> stop_if_not -> <Anonymous> -> %in%

An offtopic question at the end. How did you come across this project?

ColinFay commented 6 years ago

Hey,

So, I'll guess I'll answer point by point:

  1. devtools_history is a file I create to keep trace of all the dev related action I've performed. That way when you receive you have a direct file to refer to for all the things that could otherwise be run in the console. Feel free to drop it if you want :)

  2. No pblm :) Here are the new function names I suggest : is_swearword, pct_swearword, n_swearword, any_swearword, censor_swearword. I've kept them singular, as it seems to me important to keep some kind of consistency between function names. Tell me if this sounds ok to you.

  3. Done

  4. Done

  5. {attempt} is a package of mine :) Low on dependencies (just has {rlang}, which in return has zero dep), so the dependency tree is pretty small. I never remember how to use base::stopifnot, and when I do, it seems a little bit cumbersome to write something to catch if it's an error, and then writing a use message. This is what is done directly in attempt::stop_if_not : test, and if the condition is not met, the error message specified is returned. Let me know if you'd prefer me to rewrite the check function, no pblm.

  6. Arf, noob error: I forgot to push a file :/ I have a vector called swear_lang that contains the unique of the languages. It now appears to me that it is only called once in the check function, so I've put the unique there instead.

And for the last question, I stumbled upon your repo as I was doing some text mining, and was looking for a "bad word labeller". I coded some functions upon the package, so it seemed natural to me to send these one to you.

pdrhlik commented 6 years ago

Hi @ColinFay, sorry for the delayed reply.

Point by point to you again.

  1. I would drop it for now if you don't mind :-)
  2. This is a lot better. I definitely want the function names to be consistent. I'm just thinking that some of them might be plural. I would only keep is_swearword singular and rename others to include the plural "s" at the end. It makes sense even if you take a look at the function's arguments. is_swearword works with a single word but all of the others work with vectors. Does it make sense to you too?
  3. OK
  4. OK
  5. I've noticed {attempt} is yours, it's pretty cool :-) Although if you don't mind, I would prefer not to include it for now. I'll keep it in mind for the future though!
  6. OK

And one more note. I'd prefer not to change the development version number yet. We're figuring out what should be included in the first version but until then it's pointless to change it IMO.